Fixing gcc 4.7 building problems. (issue 10451068)

[she...@google.com]

Reviewers: dkegel, Elliot Glaysher, tbarzic, Julian Pastarmov, brettw at
google, Mattias Nissler, agl2, asharif,

Message:
Hi guys, mind taking a look at this CL? Thanks.

Description:
Fixing gcc 4.7 building problems.

a) - gcc-4.7 improved the implicit headers that it includes. with
<4.7, the gthr-default.h file always pulls in unistd.h. with >=4.7,
they avoided that include when possible. so code that isn't including
unistd.h itself but needs it now breaks.

b) - narrowing conversion in initiliazation list now raises an
'ill-formed conversion' warning, which causes error when -Werror is
given.

c) - included patches from pastebin - http://pastebin.com/raw.php?i=p3UKs7Cg

Note - this may not be fixing all the gcc 4.7 build problems for all
parts, but rather than submitting one big-fix-for-all CL, we'd better
do it incrementally (given that all the modification is reasonable and
minor) so that at least some parts get a successful gcc 4.7 build.

BUG=None
TEST=Built successfully using GCC-4.7 under chromium chroot


Please review this at https://chromiumcodereview.appspot.com/10451068/

SVN Base: http://git.chromium.org/chromium/src.git@master

Affected files:
M base/time_posix.cc
M chrome/browser/chromeos/process_proxy/process_output_watcher.cc
M chrome/browser/extensions/settings/settings_frontend.cc
M chrome/browser/policy/policy_path_parser_linux.cc
M chrome/browser/search_engines/template_url_prepopulate_data.h
M content/public/common/sandbox_init.cc
M crypto/ec_private_key_nss.cc
M crypto/ec_signature_creator_nss.cc
M crypto/third_party/nss/secsign.cc
M gpu/command_buffer/common/types.h
M ipc/ipc_channel.h
M ipc/ipc_channel_posix.cc
M ipc/ipc_platform_file.cc
M net/base/x509_util_nss.cc
M ppapi/tests/test_broker.cc
M third_party/tcmalloc/chromium/src/base/stl_allocator.h
M third_party/tcmalloc/chromium/src/base/vdso_support.h
M third_party/tcmalloc/chromium/src/common.cc
M third_party/tcmalloc/chromium/src/symbolize.h
M ui/gfx/skia_utils_gtk.cc
M webkit/glue/webcursor_gtk_data.h
M webkit/plugins/ppapi/ppb_flash_impl.cc
M webkit/plugins/ppapi/ppb_video_capture_impl.cc


Index: base/time_posix.cc
diff --git a/base/time_posix.cc b/base/time_posix.cc
index
18203cf80bf7eb7b3fa9fddf797783ee0b1420c6..ce1e87e9c27ff38406a431e74c7372b8ca35e664
100644
--- a/base/time_posix.cc
+++ b/base/time_posix.cc
@@ -34,7 +34,7 @@ struct timespec TimeDelta::ToTimeSpec() const {
}
struct timespec result =
{seconds,
- microseconds * Time::kNanosecondsPerMicrosecond};
+ static_cast<long int>(microseconds *
Time::kNanosecondsPerMicrosecond)};
return result;
}

Index: chrome/browser/chromeos/process_proxy/process_output_watcher.cc
diff --git
a/chrome/browser/chromeos/process_proxy/process_output_watcher.cc
b/chrome/browser/chromeos/process_proxy/process_output_watcher.cc
index
559b18382ed42a06aebeb3e7347ab217adb54b25..7e12675419de137a636fbe483a3b637ca9fc4506
100644
--- a/chrome/browser/chromeos/process_proxy/process_output_watcher.cc
+++ b/chrome/browser/chromeos/process_proxy/process_output_watcher.cc
@@ -11,6 +11,8 @@
#include <sys/ioctl.h>
#include <sys/select.h>

+#include <unistd.h>
+
#include "base/eintr_wrapper.h"
#include "base/logging.h"

Index: chrome/browser/extensions/settings/settings_frontend.cc
diff --git a/chrome/browser/extensions/settings/settings_frontend.cc
b/chrome/browser/extensions/settings/settings_frontend.cc
index
f20e58d9ee210860ea31f753b45192954df5b690..9af54d525f652463c7ddb54e1238d66aee5ef1d3
100644
--- a/chrome/browser/extensions/settings/settings_frontend.cc
+++ b/chrome/browser/extensions/settings/settings_frontend.cc
@@ -99,9 +99,9 @@ SettingsStorageQuotaEnforcer::Limits GetLocalLimits() {

SettingsStorageQuotaEnforcer::Limits GetSyncLimits() {
SettingsStorageQuotaEnforcer::Limits limits = {
- api::storage::sync::QUOTA_BYTES,
- api::storage::sync::QUOTA_BYTES_PER_ITEM,
- api::storage::sync::MAX_ITEMS
+ static_cast<size_t>(api::storage::sync::QUOTA_BYTES),
+ static_cast<size_t>(api::storage::sync::QUOTA_BYTES_PER_ITEM),
+ static_cast<size_t>(api::storage::sync::MAX_ITEMS)
};
return limits;
}
Index: chrome/browser/policy/policy_path_parser_linux.cc
diff --git a/chrome/browser/policy/policy_path_parser_linux.cc
b/chrome/browser/policy/policy_path_parser_linux.cc
index
2f9ea26b5df1e0c952da2570c43ad81bcc47ba7e..cddd58b5b328cb4c7547652369d7411907625120
100644
--- a/chrome/browser/policy/policy_path_parser_linux.cc
+++ b/chrome/browser/policy/policy_path_parser_linux.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.

#include <pwd.h>
+#include <unistd.h>

#include "chrome/browser/policy/policy_path_parser.h"

Index: chrome/browser/search_engines/template_url_prepopulate_data.h
diff --git a/chrome/browser/search_engines/template_url_prepopulate_data.h
b/chrome/browser/search_engines/template_url_prepopulate_data.h
index
dc989f9826ab91398905593dbe1e56d9fbe85576..101f5d4a75b584fef7e9a1b5ba56796035b53747
100644
--- a/chrome/browser/search_engines/template_url_prepopulate_data.h
+++ b/chrome/browser/search_engines/template_url_prepopulate_data.h
@@ -9,6 +9,7 @@
#include <stddef.h>
#include <string>
#include <vector>
+#include <cstddef>

#include "base/string16.h"
#include "chrome/browser/search_engines/search_engine_type.h"
Index: content/public/common/sandbox_init.cc
diff --git a/content/public/common/sandbox_init.cc
b/content/public/common/sandbox_init.cc
index
528eec7e314c4dfd876092e11336aa89142be176..f80b8566103f2d77311e87a910a763d71bfcd0b1
100644
--- a/content/public/common/sandbox_init.cc
+++ b/content/public/common/sandbox_init.cc
@@ -4,7 +4,7 @@

#include "content/public/common/sandbox_init.h"

-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_LINUX)
#include <unistd.h>
#endif

Index: crypto/ec_private_key_nss.cc
diff --git a/crypto/ec_private_key_nss.cc b/crypto/ec_private_key_nss.cc
index
1fb13e7e9ca91d8673ac397b00bd24dffd3e91b2..fa76e4932abb3f9af551119c99b3fe29d7cd5985
100644
--- a/crypto/ec_private_key_nss.cc
+++ b/crypto/ec_private_key_nss.cc
@@ -128,7 +128,7 @@ bool ECPrivateKey::ImportFromEncryptedPrivateKeyInfo(
SECItem encoded_epki = {
siBuffer,
const_cast<unsigned char*>(encrypted_private_key_info),
- encrypted_private_key_info_len
+ static_cast<unsigned int>(encrypted_private_key_info_len)
};
SECKEYEncryptedPrivateKeyInfo epki;
memset(&epki, 0, sizeof(epki));
@@ -150,7 +150,7 @@ bool ECPrivateKey::ImportFromEncryptedPrivateKeyInfo(
SECItem password_item = {
siBuffer,
reinterpret_cast<unsigned char*>(const_cast<char*>(password.data())),
- password.size()
+ static_cast<unsigned int>(password.size())
};

rv = ImportEncryptedECPrivateKeyInfoAndReturnKey(
@@ -185,7 +185,7 @@ bool ECPrivateKey::ExportEncryptedPrivateKey(
SECItem password_item = {
siBuffer,
reinterpret_cast<unsigned char*>(const_cast<char*>(password.data())),
- password.size()
+ static_cast<unsigned int>(password.size())
};

SECKEYEncryptedPrivateKeyInfo* encrypted =
PK11_ExportEncryptedPrivKeyInfo(
@@ -264,7 +264,8 @@ ECPrivateKey* ECPrivateKey::CreateWithParams(bool
permanent,
DCHECK_LE(oid_data->oid.len, 127U);
std::vector<unsigned char> parameters_buf(2 + oid_data->oid.len);
SECKEYECParams ec_parameters = {
- siDEROID, &parameters_buf[0], parameters_buf.size()
+ siDEROID, &parameters_buf[0],
+ static_cast<unsigned int>(parameters_buf.size())
};

ec_parameters.data[0] = SEC_ASN1_OBJECT_ID;
@@ -300,7 +301,7 @@ ECPrivateKey*
ECPrivateKey::CreateFromEncryptedPrivateKeyInfoWithParams(
SECItem encoded_spki = {
siBuffer,
const_cast<unsigned char*>(&subject_public_key_info[0]),
- subject_public_key_info.size()
+ static_cast<unsigned int>(subject_public_key_info.size())
};
CERTSubjectPublicKeyInfo* decoded_spki =
SECKEY_DecodeDERSubjectPublicKeyInfo(
&encoded_spki);
Index: crypto/ec_signature_creator_nss.cc
diff --git a/crypto/ec_signature_creator_nss.cc
b/crypto/ec_signature_creator_nss.cc
index
388870f7fa8cf5aa6f792a3326f8227fe7d0a74a..21c192fa173f81266de5202e827d97a222ca77e4
100644
--- a/crypto/ec_signature_creator_nss.cc
+++ b/crypto/ec_signature_creator_nss.cc
@@ -8,6 +8,7 @@
#include <pk11pub.h>
#include <secerr.h>
#include <sechash.h>
+#include <unistd.h>

#include "base/logging.h"
#include "crypto/ec_private_key.h"
@@ -34,12 +35,14 @@ SECStatus SignData(SECItem* result,
hash_type, &hash_data[0], input->data, input->len);
if (rv != SECSuccess)
return rv;
- SECItem hash = {siBuffer, &hash_data[0], hash_data.size()};
+ SECItem hash = {siBuffer, &hash_data[0],
+ static_cast<unsigned int>(hash_data.size())};

// Compute signature of hash.
int signature_len = PK11_SignatureLen(key);
std::vector<uint8> signature_data(signature_len);
- SECItem sig = {siBuffer, &signature_data[0], signature_len};
+ SECItem sig = {siBuffer, &signature_data[0],
+ static_cast<unsigned int>(signature_len)};
rv = PK11_Sign(key, &sig, &hash);
if (rv != SECSuccess)
return rv;
Index: crypto/third_party/nss/secsign.cc
diff --git a/crypto/third_party/nss/secsign.cc
b/crypto/third_party/nss/secsign.cc
index
9272d4a3b5a7b349647ffff723069c2afb724f93..a788defc70410559d0546299802b0a724eaee63f
100644
--- a/crypto/third_party/nss/secsign.cc
+++ b/crypto/third_party/nss/secsign.cc
@@ -93,12 +93,14 @@ SECStatus DerSignData(PLArenaPool *arena,
hash_type, &hash_data[0], input->data, input->len);
if (rv != SECSuccess)
return rv;
- SECItem hash = {siBuffer, &hash_data[0], hash_data.size()};
+ SECItem hash = {siBuffer, &hash_data[0],
+ static_cast<unsigned int>(hash_data.size())};

// Compute signature of hash.
int signature_len = PK11_SignatureLen(key);
std::vector<uint8> signature_data(signature_len);
- SECItem sig = {siBuffer, &signature_data[0], signature_len};
+ SECItem sig = {siBuffer, &signature_data[0],
+ static_cast<unsigned int>(signature_len)};
rv = PK11_Sign(key, &sig, &hash);
if (rv != SECSuccess)
return rv;
Index: gpu/command_buffer/common/types.h
diff --git a/gpu/command_buffer/common/types.h
b/gpu/command_buffer/common/types.h
index
718ecca6763b86d07837ce2c32834baf25b941cc..76d0cb23e9fc75f36504913c2db47fcb70dd2c9b
100644
--- a/gpu/command_buffer/common/types.h
+++ b/gpu/command_buffer/common/types.h
@@ -12,6 +12,7 @@
#endif
#include <cstddef>
#include <string>
+#include <cstddef>

typedef signed char schar;
typedef signed char int8;
Index: ipc/ipc_channel.h
diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h
index
2aeca1fceda3a79ee34e080fb60e2487e404203e..19f4c822af6dd6979ac3258b18e91bd4f52de856
100644
--- a/ipc/ipc_channel.h
+++ b/ipc/ipc_channel.h
@@ -7,6 +7,7 @@
#pragma once

#include <string>
+#include <sys/types.h>

#include "base/compiler_specific.h"
#include "base/process.h"
Index: ipc/ipc_channel_posix.cc
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index
9d9b8d19990e87164a28a356729ec6eca470f5ac..3f759dfee98519a1d9365c7207ef781a53146915
100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -18,6 +18,7 @@

#include <string>
#include <map>
+#include <unistd.h>

#include "base/command_line.h"
#include "base/eintr_wrapper.h"
@@ -898,7 +899,7 @@ Channel::ChannelImpl::ReadState
Channel::ChannelImpl::ReadData(

struct msghdr msg = {0};

- struct iovec iov = {buffer, buffer_len};
+ struct iovec iov = {buffer, static_cast<size_t>(buffer_len)};
msg.msg_iov = &iov;
msg.msg_iovlen = 1;

Index: ipc/ipc_platform_file.cc
diff --git a/ipc/ipc_platform_file.cc b/ipc/ipc_platform_file.cc
index
b5ec7be0d94919726854f52e4edca36ae7ec2804..6d7cf0f7b7dc209d7d4ecfeab13c4d94f6b61087
100644
--- a/ipc/ipc_platform_file.cc
+++ b/ipc/ipc_platform_file.cc
@@ -4,7 +4,7 @@

#include "ipc/ipc_platform_file.h"

-#if defined(OS_ANDROID)
+#if defined(OS_LINUX)
#include <unistd.h>
#endif

Index: net/base/x509_util_nss.cc
diff --git a/net/base/x509_util_nss.cc b/net/base/x509_util_nss.cc
index
08cd7e94fb2653cf65a83d792a095d64131a4cd8..e08cccca23e5a3e66c7991d6ef3da923d77d87ca
100644
--- a/net/base/x509_util_nss.cc
+++ b/net/base/x509_util_nss.cc
@@ -196,7 +196,7 @@ bool CreateDomainBoundCertInternal(
SECItem domain_string_item = {
siAsciiString,
(unsigned char*)domain.data(),
- domain.size()
+ static_cast<unsigned int>(domain.size())
};

// IA5Encode and arena allocate SECItem
Index: ppapi/tests/test_broker.cc
diff --git a/ppapi/tests/test_broker.cc b/ppapi/tests/test_broker.cc
index
db75def4f3602f43f30004bccdc055549e819949..aa3969857b9a54d070a5ba7638719d8bdbb62214
100644
--- a/ppapi/tests/test_broker.cc
+++ b/ppapi/tests/test_broker.cc
@@ -16,6 +16,7 @@
#include <cstring>
#include <fstream>
#include <limits>
+#include <unistd.h>

#include "ppapi/c/pp_errors.h"
#include "ppapi/c/trusted/ppp_broker.h"
Index: third_party/tcmalloc/chromium/src/base/stl_allocator.h
diff --git a/third_party/tcmalloc/chromium/src/base/stl_allocator.h
b/third_party/tcmalloc/chromium/src/base/stl_allocator.h
index
f7adb687e627a43b165854fe8f09f34f24fbc14e..824988b0b59f1f12221d00b74816ec4ba1a2655a
100644
--- a/third_party/tcmalloc/chromium/src/base/stl_allocator.h
+++ b/third_party/tcmalloc/chromium/src/base/stl_allocator.h
@@ -39,6 +39,7 @@

#include <stddef.h> // for std::ptrdiff_t
#include <limits>
+#include <cstddef>

#include "base/logging.h"

Index: third_party/tcmalloc/chromium/src/base/vdso_support.h
diff --git a/third_party/tcmalloc/chromium/src/base/vdso_support.h
b/third_party/tcmalloc/chromium/src/base/vdso_support.h
index
b97ab254d38eac51c6158c0203d0a19d37487ef1..a839441be25cb3b2af6ae34123c016ed2d4dca14
100644
--- a/third_party/tcmalloc/chromium/src/base/vdso_support.h
+++ b/third_party/tcmalloc/chromium/src/base/vdso_support.h
@@ -63,6 +63,7 @@

#define HAVE_VDSO_SUPPORT 1

+#include <cstddef>
#include <stdlib.h> // for NULL

namespace base {
Index: third_party/tcmalloc/chromium/src/common.cc
diff --git a/third_party/tcmalloc/chromium/src/common.cc
b/third_party/tcmalloc/chromium/src/common.cc
index
dd175f223692a5adb37048140b30422bae79fe40..fa88511c83d953d0b22069ddc31684fd15904449
100644
--- a/third_party/tcmalloc/chromium/src/common.cc
+++ b/third_party/tcmalloc/chromium/src/common.cc
@@ -33,6 +33,7 @@
#include "config.h"
#include "common.h"
#include "system-alloc.h"
+#include <cstddef>

#if defined(HAVE_UNISTD_H) && defined(HAVE_GETPAGESIZE)
#include <unistd.h> // for getpagesize
Index: third_party/tcmalloc/chromium/src/symbolize.h
diff --git a/third_party/tcmalloc/chromium/src/symbolize.h
b/third_party/tcmalloc/chromium/src/symbolize.h
index
da070378b11676693a1bf519256fa04e5fc68ac5..089a185aa83ab65953495056685671b0a3c8b8b5
100644
--- a/third_party/tcmalloc/chromium/src/symbolize.h
+++ b/third_party/tcmalloc/chromium/src/symbolize.h
@@ -40,6 +40,7 @@
#endif
#include <stddef.h> // for NULL
#include <map>
+#include <cstddef>

using std::map;

Index: ui/gfx/skia_utils_gtk.cc
diff --git a/ui/gfx/skia_utils_gtk.cc b/ui/gfx/skia_utils_gtk.cc
index
8c6f455e05952754221181876b93225023b548db..f6592479848c5bec0286d9ed9f9164f1de6c1f0e
100644
--- a/ui/gfx/skia_utils_gtk.cc
+++ b/ui/gfx/skia_utils_gtk.cc
@@ -22,9 +22,9 @@ SkColor GdkColorToSkColor(GdkColor color) {
GdkColor SkColorToGdkColor(SkColor color) {
GdkColor gdk_color = {
0,
- SkColorGetR(color) * kSkiaToGDKMultiplier,
- SkColorGetG(color) * kSkiaToGDKMultiplier,
- SkColorGetB(color) * kSkiaToGDKMultiplier
+ static_cast<guint16>(SkColorGetR(color) * kSkiaToGDKMultiplier),
+ static_cast<guint16>(SkColorGetG(color) * kSkiaToGDKMultiplier),
+ static_cast<guint16>(SkColorGetB(color) * kSkiaToGDKMultiplier)
};
return gdk_color;
}
Index: webkit/glue/webcursor_gtk_data.h
diff --git a/webkit/glue/webcursor_gtk_data.h
b/webkit/glue/webcursor_gtk_data.h
index
a9831433450f9b00e24005ac25e430a07bdcc683..98624c50784c3eec0656833ef5e4af22f154efac
100644
--- a/webkit/glue/webcursor_gtk_data.h
+++ b/webkit/glue/webcursor_gtk_data.h
@@ -31,6 +31,8 @@
http://lxr.mozilla.org/mozilla1.8/source/widget/src/gtk2/nsGtkCursors.h
*/

+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wnarrowing"
/* MOZ_CURSOR_VERTICAL_TEXT */
static const char moz_vertical_text_bits[] = {
0x00, 0x00, 0x00, 0x00, 0x02, 0x40, 0x00, 0x00, 0x02, 0x40, 0x00, 0x00,
@@ -309,3 +311,4 @@ static const char moz_spinning_mask_bits[] = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
+#pragma GCC diagnostic pop
Index: webkit/plugins/ppapi/ppb_flash_impl.cc
diff --git a/webkit/plugins/ppapi/ppb_flash_impl.cc
b/webkit/plugins/ppapi/ppb_flash_impl.cc
index
668a8c13b36d393546074ce9c12874be9e239e26..9290ba06ea3e3be7fc7f2acac1380b5f590335ad
100644
--- a/webkit/plugins/ppapi/ppb_flash_impl.cc
+++ b/webkit/plugins/ppapi/ppb_flash_impl.cc
@@ -124,9 +124,10 @@ PP_Bool PPB_Flash_Impl::DrawGlyphs(PP_Instance
instance,
SkAutoCanvasRestore acr(canvas, true);

// Clip is applied in pixels before the transform.
- SkRect clip_rect = { clip->point.x, clip->point.y,
- clip->point.x + clip->size.width,
- clip->point.y + clip->size.height };
+ SkRect clip_rect = { SkIntToScalar(clip->point.x),
+ SkIntToScalar(clip->point.y),
+ SkIntToScalar(clip->point.x + clip->size.width),
+ SkIntToScalar(clip->point.y + clip->size.height) };
canvas->clipRect(clip_rect);

// Convert & set the matrix.
Index: webkit/plugins/ppapi/ppb_video_capture_impl.cc
diff --git a/webkit/plugins/ppapi/ppb_video_capture_impl.cc
b/webkit/plugins/ppapi/ppb_video_capture_impl.cc
index
f1eb12c9b489e45723ac696f75a853e6b71d3665..9a00155ee0e72653d6a12f2628e3a8712153dff4
100644
--- a/webkit/plugins/ppapi/ppb_video_capture_impl.cc
+++ b/webkit/plugins/ppapi/ppb_video_capture_impl.cc
@@ -118,9 +118,9 @@ void PPB_VideoCapture_Impl::OnDeviceInfoReceived(
media::VideoCapture* capture,
const media::VideoCaptureParams& device_info) {
PP_VideoCaptureDeviceInfo_Dev info = {
- device_info.width,
- device_info.height,
- device_info.frame_per_second
+ static_cast<uint32_t>(device_info.width),
+ static_cast<uint32_t>(device_info.height),
+ static_cast<uint32_t>(device_info.frame_per_second)
};
ReleaseBuffers();

[bre...@chromium.org]

Please list who you would like to review what.

https://chromiumcodereview.appspot.com/10451068/

[a...@chromium.org]

crypto/, ipc/ LGTM


https://chromiumcodereview.appspot.com/10451068/diff/2001/ipc/ipc_channel.h
File ipc/ipc_channel.h (right):

https://chromiumcodereview.appspot.com/10451068/diff/2001/ipc/ipc_channel.h#newcode10
ipc/ipc_channel.h:10: #include <sys/types.h>
Wrap with #if defined(OS_POSIX)?

https://chromiumcodereview.appspot.com/10451068/diff/2001/ipc/ipc_platform_file.cc
File ipc/ipc_platform_file.cc (right):

https://chromiumcodereview.appspot.com/10451068/diff/2001/ipc/ipc_platform_file.cc#newcode7
ipc/ipc_platform_file.cc:7: #if defined(OS_LINUX)
OS_POSIX again?

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

Thanks, Adam.

https://chromiumcodereview.appspot.com/10451068/diff/2001/ipc/ipc_channel.h
File ipc/ipc_channel.h (right):

https://chromiumcodereview.appspot.com/10451068/diff/2001/ipc/ipc_channel.h#newcode10
ipc/ipc_channel.h:10: #include <sys/types.h>

On 2012/05/29 18:33:22, agl wrote:
> Wrap with #if defined(OS_POSIX)?

Done.

https://chromiumcodereview.appspot.com/10451068/diff/2001/ipc/ipc_platform_file.cc
File ipc/ipc_platform_file.cc (right):

https://chromiumcodereview.appspot.com/10451068/diff/2001/ipc/ipc_platform_file.cc#newcode7
ipc/ipc_platform_file.cc:7: #if defined(OS_LINUX)

On 2012/05/29 18:33:22, agl wrote:
> OS_POSIX again?

Done.

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

On 2012/05/29 18:23:19, brettw wrote:
> Please list who you would like to review what.

Yeah, listed reviewers/parts below -

brettw -
base/...
chrome/...
content/

agl -
crypto/
ipc/
net/

jamesr -
webkit/

sky -
ui/
third_party/tcmalloc/

yzshen
- ppapi/


https://chromiumcodereview.appspot.com/10451068/

[bre...@chromium.org]

LGTM


https://chromiumcodereview.appspot.com/10451068/diff/9001/base/time_posix.cc
File base/time_posix.cc (right):

https://chromiumcodereview.appspot.com/10451068/diff/9001/base/time_posix.cc#newcode37
base/time_posix.cc:37: static_cast<long int>(microseconds *
Time::kNanosecondsPerMicrosecond)};
Can you just do "long" here (it's the same, right?)

https://chromiumcodereview.appspot.com/10451068/diff/9001/chrome/browser/chromeos/process_proxy/process_output_watcher.cc
File chrome/browser/chromeos/process_proxy/process_output_watcher.cc
(right):

https://chromiumcodereview.appspot.com/10451068/diff/9001/chrome/browser/chromeos/process_proxy/process_output_watcher.cc#newcode13
chrome/browser/chromeos/process_proxy/process_output_watcher.cc:13:
No blank line here (all C includes to together).

https://chromiumcodereview.appspot.com/10451068/diff/9001/chrome/browser/search_engines/template_url_prepopulate_data.h
File chrome/browser/search_engines/template_url_prepopulate_data.h
(right):

https://chromiumcodereview.appspot.com/10451068/diff/9001/chrome/browser/search_engines/template_url_prepopulate_data.h#newcode12
chrome/browser/search_engines/template_url_prepopulate_data.h:12:
#include <cstddef>
C++ includes should go together and be in order (so above <string>)

https://chromiumcodereview.appspot.com/10451068/diff/9001/crypto/ec_private_key_nss.cc
File crypto/ec_private_key_nss.cc (right):

https://chromiumcodereview.appspot.com/10451068/diff/9001/crypto/ec_private_key_nss.cc#newcode131
crypto/ec_private_key_nss.cc:131: static_cast<unsigned
int>(encrypted_private_key_info_len)
unsigned int -> unsigned (same elsewhere)

https://chromiumcodereview.appspot.com/10451068/

[Jochen Eisinger]

Where does the pastebin diff come from, esp. who holds the copyright?

[she...@google.com]

Hi Brett, thanks. Done.

https://chromiumcodereview.appspot.com/10451068/diff/9001/base/time_posix.cc
File base/time_posix.cc (right):

https://chromiumcodereview.appspot.com/10451068/diff/9001/base/time_posix.cc#newcode37
base/time_posix.cc:37: static_cast<long int>(microseconds *
Time::kNanosecondsPerMicrosecond)};

On 2012/05/29 19:30:46, brettw wrote:
> Can you just do "long" here (it's the same, right?)

Yes, 'long' is just an alias for 'long int'.

https://chromiumcodereview.appspot.com/10451068/diff/9001/chrome/browser/chromeos/process_proxy/process_output_watcher.cc
File chrome/browser/chromeos/process_proxy/process_output_watcher.cc
(right):

https://chromiumcodereview.appspot.com/10451068/diff/9001/chrome/browser/chromeos/process_proxy/process_output_watcher.cc#newcode13
chrome/browser/chromeos/process_proxy/process_output_watcher.cc:13:

On 2012/05/29 19:30:46, brettw wrote:
> No blank line here (all C includes to together).

Done.

https://chromiumcodereview.appspot.com/10451068/diff/9001/chrome/browser/search_engines/template_url_prepopulate_data.h
File chrome/browser/search_engines/template_url_prepopulate_data.h
(right):

https://chromiumcodereview.appspot.com/10451068/diff/9001/chrome/browser/search_engines/template_url_prepopulate_data.h#newcode12
chrome/browser/search_engines/template_url_prepopulate_data.h:12:
#include <cstddef>

On 2012/05/29 19:30:46, brettw wrote:
> C++ includes should go together and be in order (so above <string>)

Done.

https://chromiumcodereview.appspot.com/10451068/diff/9001/crypto/ec_private_key_nss.cc
File crypto/ec_private_key_nss.cc (right):

https://chromiumcodereview.appspot.com/10451068/diff/9001/crypto/ec_private_key_nss.cc#newcode131
crypto/ec_private_key_nss.cc:131: static_cast<unsigned
int>(encrypted_private_key_info_len)

On 2012/05/29 19:30:46, brettw wrote:
> unsigned int -> unsigned (same elsewhere)

Done.

https://chromiumcodereview.appspot.com/10451068/

[yzs...@chromium.org]

ppapi/ LGTM
Thanks!

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

On 2012/05/29 19:37:33, jochen wrote:
> Where does the pastebin diff come from, esp. who holds the copyright?

> On May 29, 2012 9:30 PM, <mailto:bre...@chromium.org> wrote:

Hi John, I think these are reasonable/simple/straightforward fixes.

But Andres could you answer John's the question?

Thanks,
-Han

https://chromiumcodereview.appspot.com/10451068/

[mnis...@chromium.org]

LGTM with a nit.


https://chromiumcodereview.appspot.com/10451068/diff/7002/chrome/browser/policy/policy_path_parser_linux.cc
File chrome/browser/policy/policy_path_parser_linux.cc (right):

https://chromiumcodereview.appspot.com/10451068/diff/7002/chrome/browser/policy/policy_path_parser_linux.cc#newcode6
chrome/browser/policy/policy_path_parser_linux.cc:6: #include <unistd.h>
I guess this one is for geteuid? While you're at it, can you also add
the missing sys/types.h (for geteuid and getpwuid)?

https://chromiumcodereview.appspot.com/10451068/

[Jochen Eisinger]

On Wed, May 30, 2012 at 3:33 PM, <andrea...@gmail.com> wrote:

On 2012/05/29 21:01:53, Han wrote:

On 2012/05/29 19:37:33, jochen wrote:
> Where does the pastebin diff come from, esp. who holds the copyright?
> On May 29, 2012 9:30 PM, <mailto:bre...@chromium.org> wrote:
>
Hi John, I think these are reasonable/simple/straightforward fixes.

But Andres could you answer John's the question?

Thanks,
-Han

The <cstddef>s came from the Arch Linux build script, AFAIK introduced by Pierre
Schmitz here:
https://projects.archlinux.org/svntogit/packages.git/commit/trunk?h=packages/chromium&id=b18a2979663cbb653fac9e6e5c04e8ef7631bc68

The other parts of the pastebin diff were added by me. Since they are trivial I
doubt that they warrant a copyright.

I don't know from what point on the rules apply, but I can't find either of you in the CLA signers list

Adding mal for clarification

 

(sandbox_init.cc might be better of with "(OS_POSIX)" instead of "(OS_ANDROID)
|| defined(OS_LINUX)", like ipc_platform_file.cc, esp. since both files closely
resemble each other.)

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

Mattias, thanks, done!

https://chromiumcodereview.appspot.com/10451068/diff/7002/chrome/browser/policy/policy_path_parser_linux.cc
File chrome/browser/policy/policy_path_parser_linux.cc (right):

https://chromiumcodereview.appspot.com/10451068/diff/7002/chrome/browser/policy/policy_path_parser_linux.cc#newcode6
chrome/browser/policy/policy_path_parser_linux.cc:6: #include <unistd.h>

On 2012/05/30 08:02:25, Mattias Nissler wrote:
> I guess this one is for geteuid? While you're at it, can you also add
the
> missing sys/types.h (for geteuid and getpwuid)?

Yeah, right, both the headers are required for geteuid.
Done.

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

On 2012/05/29 18:44:35, Han wrote:
> On 2012/05/29 18:23:19, brettw wrote:
> > Please list who you would like to review what.

> Yeah, listed reviewers/parts below -

> brettw -
> base/...
> chrome/...
> content/

> agl -
> crypto/
> ipc/
> net/

> jamesr -
> webkit/

> sky -
> ui/
> third_party/tcmalloc/

> yzshen
> - ppapi/

Hi James, could you take a look at "webkit/"? Thanks!

https://chromiumcodereview.appspot.com/10451068/

[jam...@chromium.org]

The CLA situation should be sorted out before spending time reviewing.

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

On 2012/05/31 17:11:28, jamesr wrote:
> The CLA situation should be sorted out before spending time reviewing.

Hi James, the modifications under "webkit/" is solely contributed by me, it
is
not part of "pastebin". I'll revert modifications from "pastebin" if CLA is
not
sorted out by tomorrow. Thanks,

-Han

https://chromiumcodereview.appspot.com/10451068/

[jam...@chromium.org]

https://chromiumcodereview.appspot.com/10451068/diff/2004/webkit/glue/webcursor_gtk_data.h
File webkit/glue/webcursor_gtk_data.h (right):

https://chromiumcodereview.appspot.com/10451068/diff/2004/webkit/glue/webcursor_gtk_data.h#newcode35
webkit/glue/webcursor_gtk_data.h:35: #pragma GCC diagnostic ignored
"-Wnarrowing"
This seems very weird to me - we normally control compilation flags from
the build system, not inline in the code. I can't find any other
instances of this sort of thing today - why can't you set this in the
gypi?

https://chromiumcodereview.appspot.com/10451068/

[m...@google.com]

I would strongly prefer that the original authors of the code sign the CLA
to
make it clear that they're giving the Chromium Authors rights to their
creative
work. Our rights to use someone else's work don't depend on
complexity/triviality.

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

On 2012/05/31 18:24:20, mal wrote:
> I would strongly prefer that the original authors of the code sign the
> CLA to
> make it clear that they're giving the Chromium Authors rights to their
creative
> work. Our rights to use someone else's work don't depend on
> complexity/triviality.

Hi Mark, thanks! I'll revert modifications from pastebin.

-Han

https://chromiumcodereview.appspot.com/10451068/

[m...@google.com]

We should at least get a statement from the authors that the changes are ok
for
us to use if the CLA isn't signed.

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

Hi James thanks! Moved the flag change to webkit_glue.gypi.

-Han


https://chromiumcodereview.appspot.com/10451068/

[jam...@chromium.org]

webkit/ LGTM


https://chromiumcodereview.appspot.com/10451068/diff/5004/webkit/glue/webcursor_gtk_data.h
File webkit/glue/webcursor_gtk_data.h (right):

https://chromiumcodereview.appspot.com/10451068/diff/5004/webkit/glue/webcursor_gtk_data.h#newcode312
webkit/glue/webcursor_gtk_data.h:312:
spurious newline here

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

On 2012/05/31 18:35:05, Andreas Reis wrote:

> On 2012/05/31 18:27:19, mal wrote:
> > We should at least get a statement from the authors that the changes
> are ok
> for
> > us to use if the CLA isn't signed.

> As for my parts, use them as you like; I give you any rights whatsoever
> necessary…

> Can't speak for Pierre Schmitz, of course.

Also note, I've reverted the changes from pastebin.

Hi Andreas, no worry, I'll upload another bunch of changes regarding gcc 4.7
changes, you can send me YOUR patches. Of course you can upload it for
review by
yourself, it's easy.

Thanks,
-Han

https://chromiumcodereview.appspot.com/10451068/

[she...@google.com]

https://chromiumcodereview.appspot.com/10451068/diff/5004/webkit/glue/webcursor_gtk_data.h
File webkit/glue/webcursor_gtk_data.h (right):

https://chromiumcodereview.appspot.com/10451068/diff/5004/webkit/glue/webcursor_gtk_data.h#newcode312
webkit/glue/webcursor_gtk_data.h:312:

On 2012/05/31 20:34:19, jamesr wrote:
> spurious newline here

Done.

https://chromiumcodereview.appspot.com/10451068/