Fix a 64-bit pointer truncation bug found by VC++ 2015

Fix a 64-bit pointer truncation bug found by VC++ 2015

When converting from void* to unsigned long long it is dangerous to go
through unsigned long because for VC++ 64-bit builds this will be 32
bits. When casting a pointer to an integral type the safest type to
choose for the integral cast is always intptr_t or uintptr_t.

BUG=440500
NOPRESUBMIT=true

Committed: https://crrev.com/952892a28a06a4ced120d4683d930699b9d730de
Cr-Commit-Position: refs/heads/master@{#10569}

[brucedawson, 2015-11-07 01:17:43]

brucedawson@chromium.org changed reviewers:
+ henrika@webrtc.org

[brucedawson, 2015-11-07 01:17:44]

PTAL. Note that this currently includes a change to codereview.settings.

[henrika_webrtc, 2015-11-09 09:01:56]

Description was changed from

==========
Fix a 64-bit pointer truncation bug found by VC++ 2015

When converting from void* to unsigned long long it is dangerous to go
through unsigned long because for VC++ 64-bit builds this will be 32
bits. When casting a pointer to an integral type the safest type to
choose for the integral cast is always intptr_t or uintptr_t.

BUG=440500
==========

to

==========
Fix a 64-bit pointer truncation bug found by VC++ 2015

When converting from void* to unsigned long long it is dangerous to go
through unsigned long because for VC++ 64-bit builds this will be 32
bits. When casting a pointer to an integral type the safest type to
choose for the integral cast is always intptr_t or uintptr_t.

BUG=440500
==========

[henrika_webrtc, 2015-11-09 09:01:56]

henrika@webrtc.org changed reviewers:
+ kjellander@webrtc.org
- henrika@webrtc.org

[henrika_webrtc, 2015-11-09 09:02:38]

henrika@webrtc.org changed reviewers:
+ henrika@webrtc.org

[henrika_webrtc, 2015-11-09 09:02:38]

kjellander@: could you check this one. You know these parts better.

[kjellander_webrtc, 2015-11-09 09:26:06]

kjellander@webrtc.org changed reviewers:
+ henrikg@webrtc.org

[kjellander_webrtc, 2015-11-09 09:26:07]

Thanks for helping out with VS2015.

I'm adding another webrtc/base/OWNERS reviewer (henrikg) for the trace change.

https://codereview.webrtc.org/1437433002/diff/1/webrtc/.gitignore
File webrtc/.gitignore (right):

https://codereview.webrtc.org/1437433002/diff/1/webrtc/.gitignore#newcode5
webrtc/.gitignore:5: *.vcxproj.filters
This looks correct, but put please them in the root .gitignore instead.

https://codereview.webrtc.org/1437433002/diff/1/webrtc/codereview.settings
File webrtc/codereview.settings (right):

https://codereview.webrtc.org/1437433002/diff/1/webrtc/codereview.settings#ne...
webrtc/codereview.settings:1: # This file is used by gcl to get repository
specific information.
Please don't change this file. See my comment in
https://codereview.appspot.com/270690043/ for a more detailed explanation.

[Henrik Grunell WebRTC, 2015-11-09 09:37:58]

webrtc/base/trace_event.h lgtm

[brucedawson, 2015-11-09 18:36:14]

On 2015/11/09 09:26:07, kjellander (webrtc) wrote:
> https://codereview.webrtc.org/1437433002/diff/1/webrtc/.gitignore#newcode5
> webrtc/.gitignore:5: *.vcxproj.filters
> This looks correct, but put please them in the root .gitignore instead.

They are already in the root .gitignore. Putting them in this location allows
"git status" to work sanely in the context of chromium\src\third_party\webrtc
where this file *is* the root .gitignore. I can also put this in a separate CL
or omit it if desired - it's not at all critical.

>
https://codereview.webrtc.org/1437433002/diff/1/webrtc/codereview.settings#ne...
> Please don't change this file. See my comment in
> https://codereview.appspot.com/270690043/ for a more detailed explanation.

Oh absolutely, I know that I shouldn't be changing this file. Unfortunately,
despite following the instructions for getting a stand-alone webrtc (I actually
tried *both* methods) I was consistently hitting error messages when trying to
do a "git cl upload". Modifying codereview.settings and including that
modification in this CL was the only way that I could get "git cl upload" to
work.

Any idea why "git cl upload" is failing for me but, presumably, working for
others? I'm currently using a repo created with "fetch webrtc". Prior to that I
tried cloning https://chromium.googlesource.com/external/webrtc. In both cases
"git cl upload" gave me this result:

C:\src\webrtc\src\webrtc>git cl upload
Traceback (most recent call last):
  File "c:\src\depot_tools\git_cl.py", line 3676, in <module>
    sys.exit(main(sys.argv[1:]))
  File "c:\src\depot_tools\git_cl.py", line 3658, in main
    return dispatcher.execute(OptionParser(), argv)
  File "c:\src\depot_tools\subcommand.py", line 252, in execute
    return command(parser, args[1:])
  File "c:\src\depot_tools\git_cl.py", line 2370, in CMDupload
    (options, args) = parser.parse_args(args)
  File "c:\src\depot_tools\git_cl.py", line 197, in Parse
    options.similarity = git_get_branch_default('git-cl-similarity', 50)
  File "c:\src\depot_tools\git_cl.py", line 169, in git_get_branch_default
    branch = Changelist().GetBranch()
  File "c:\src\depot_tools\git_cl.py", line 655, in __init__
    settings.GetDefaultServerUrl()
  File "c:\src\depot_tools\git_cl.py", line 461, in GetDefaultServerUrl
    self.LazyUpdateIfNeeded()
  File "c:\src\depot_tools\git_cl.py", line 452, in LazyUpdateIfNeeded
    LoadCodereviewSettingsFromFile(cr_settings_file)
  File "c:\src\depot_tools\git_cl.py", line 1360, in
LoadCodereviewSettingsFromFile
    keyvals = gclient_utils.ParseCodereviewSettingsContent(fileobj.read())
  File "c:\src\depot_tools\gclient_utils.py", line 1137, in
ParseCodereviewSettingsContent
    'Failed to process settings, please fix. Content:\n\n%s' % content)
gclient_utils.Error: Failed to process settings, please fix. Content:


Creating CLs from this location is not supported!
Please create a full WebRTC checkout using 'fetch webrtc'
or by cloning https://chromium.googlesource.com/external/webrtc

Sending crash report ...
  args: ['c:\\src\\depot_tools\\git_cl.py', 'upload']
  cwd: C:\src\webrtc\src\webrtc
  exception: Failed to process settings, please fix. Content:


  host: brucedawson1-w.ad.corp.google.com
  stack:   File "c:\src\depot_tools\git_cl.py", line 3676,
  user: brucedawson
  version: 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500
A stack trace has been sent to the maintainers.

[brucedawson, 2015-11-09 18:40:42]

Oh wait, Henrik explained the problem I was in the wrong directory.

It might be worth tweaking the codereview.settings file that I was modifying to
include something about being in the correct directory since I'll bet that is an
easy mistake to make. Perhaps do that in some subsequent change?

PTAL.

[kjellander_webrtc, 2015-11-09 18:56:27]

On 2015/11/09 18:36:14, brucedawson wrote:
> On 2015/11/09 09:26:07, kjellander (webrtc) wrote:
> > https://codereview.webrtc.org/1437433002/diff/1/webrtc/.gitignore#newcode5
> > webrtc/.gitignore:5: *.vcxproj.filters
> > This looks correct, but put please them in the root .gitignore instead.
> 
> They are already in the root .gitignore. Putting them in this location allows
> "git status" to work sanely in the context of chromium\src\third_party\webrtc
> where this file *is* the root .gitignore. I can also put this in a separate CL
> or omit it if desired - it's not at all critical.

Ah, they're already present in the top-level .gitignore (which is not included
in Chromium as src/third_party/webrtc points at our subdirectory.
Well then I don't mind, but please also add
*.xcodeproj
/xcodebuild
while we're at it (or even better make a separate CL for those changes).

>
https://codereview.webrtc.org/1437433002/diff/1/webrtc/codereview.settings#ne...
> > Please don't change this file. See my comment in
> > https://codereview.appspot.com/270690043/ for a more detailed explanation.
> 
> Oh absolutely, I know that I shouldn't be changing this file. Unfortunately,
> despite following the instructions for getting a stand-alone webrtc (I
actually
> tried *both* methods) I was consistently hitting error messages when trying to
> do a "git cl upload". Modifying codereview.settings and including that
> modification in this CL was the only way that I could get "git cl upload" to
> work.
> 
> Any idea why "git cl upload" is failing for me but, presumably, working for
> others? I'm currently using a repo created with "fetch webrtc". Prior to that
I
> tried cloning https://chromium.googlesource.com/external/webrtc. In both cases
> "git cl upload" gave me this result:
> 
> C:\src\webrtc\src\webrtc>git cl upload
> Traceback (most recent call last):
>   File "c:\src\depot_tools\git_cl.py", line 3676, in <module>
>     sys.exit(main(sys.argv[1:]))
>   File "c:\src\depot_tools\git_cl.py", line 3658, in main
>     return dispatcher.execute(OptionParser(), argv)
>   File "c:\src\depot_tools\subcommand.py", line 252, in execute
>     return command(parser, args[1:])
>   File "c:\src\depot_tools\git_cl.py", line 2370, in CMDupload
>     (options, args) = parser.parse_args(args)
>   File "c:\src\depot_tools\git_cl.py", line 197, in Parse
>     options.similarity = git_get_branch_default('git-cl-similarity', 50)
>   File "c:\src\depot_tools\git_cl.py", line 169, in git_get_branch_default
>     branch = Changelist().GetBranch()
>   File "c:\src\depot_tools\git_cl.py", line 655, in __init__
>     settings.GetDefaultServerUrl()
>   File "c:\src\depot_tools\git_cl.py", line 461, in GetDefaultServerUrl
>     self.LazyUpdateIfNeeded()
>   File "c:\src\depot_tools\git_cl.py", line 452, in LazyUpdateIfNeeded
>     LoadCodereviewSettingsFromFile(cr_settings_file)
>   File "c:\src\depot_tools\git_cl.py", line 1360, in
> LoadCodereviewSettingsFromFile
>     keyvals = gclient_utils.ParseCodereviewSettingsContent(fileobj.read())
>   File "c:\src\depot_tools\gclient_utils.py", line 1137, in
> ParseCodereviewSettingsContent
>     'Failed to process settings, please fix. Content:\n\n%s' % content)
> gclient_utils.Error: Failed to process settings, please fix. Content:
> 
> 
> Creating CLs from this location is not supported!
> Please create a full WebRTC checkout using 'fetch webrtc'
> or by cloning https://chromium.googlesource.com/external/webrtc
> 
> Sending crash report ...
>   args: ['c:\\src\\depot_tools\\git_cl.py', 'upload']
>   cwd: C:\src\webrtc\src\webrtc
>   exception: Failed to process settings, please fix. Content:
> 
> 
>   host: http://brucedawson1-w.ad.corp.google.com
>   stack:   File "c:\src\depot_tools\git_cl.py", line 3676,
>   user: brucedawson
>   version: 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500
> A stack trace has been sent to the maintainers.

[brucedawson, 2015-11-09 19:07:30]

I'll do a separate CL for .gitignore changes and commit the bug fix.

[brucedawson, 2015-11-09 19:08:15]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2015-11-09 19:08:16]

The patchset sent to the CQ was uploaded after l-g-t-m from henrikg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1437433002/#ps60001 (title:
"Remove .gitignore change")

[commit-bot: I haz the power, 2015-11-09 19:08:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1437433002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1437433002/60001

[commit-bot: I haz the power, 2015-11-09 19:10:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-09 19:10:52]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/1635)

[kjellander_webrtc, 2015-11-09 19:39:49]

Description was changed from

==========
Fix a 64-bit pointer truncation bug found by VC++ 2015

When converting from void* to unsigned long long it is dangerous to go
through unsigned long because for VC++ 64-bit builds this will be 32
bits. When casting a pointer to an integral type the safest type to
choose for the integral cast is always intptr_t or uintptr_t.

BUG=440500
==========

to

==========
Fix a 64-bit pointer truncation bug found by VC++ 2015

When converting from void* to unsigned long long it is dangerous to go
through unsigned long because for VC++ 64-bit builds this will be 32
bits. When casting a pointer to an integral type the safest type to
choose for the integral cast is always intptr_t or uintptr_t.

BUG=440500
NOPRESUBMIT=true
NOTRY=true
==========

[kjellander_webrtc, 2015-11-09 19:41:45]

Description was changed from

==========
Fix a 64-bit pointer truncation bug found by VC++ 2015

When converting from void* to unsigned long long it is dangerous to go
through unsigned long because for VC++ 64-bit builds this will be 32
bits. When casting a pointer to an integral type the safest type to
choose for the integral cast is always intptr_t or uintptr_t.

BUG=440500
NOPRESUBMIT=true
NOTRY=true
==========

to

==========
Fix a 64-bit pointer truncation bug found by VC++ 2015

When converting from void* to unsigned long long it is dangerous to go
through unsigned long because for VC++ 64-bit builds this will be 32
bits. When casting a pointer to an integral type the safest type to
choose for the integral cast is always intptr_t or uintptr_t.

BUG=440500
NOPRESUBMIT=true
==========

[kjellander_webrtc, 2015-11-09 19:41:55]

On 2015/11/09 19:10:52, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   presubmit on tryserver.webrtc (JOB_FAILED,
> http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/1635)

Argh, it seems you're hitting one of the few files in our repo that has another
license.
See https://code.google.com/p/webrtc/issues/detail?id=5023 for context.

I added NOPRESUBMIT=true to the description to allow landing without the
presubmit bot.

[kjellander_webrtc, 2015-11-09 19:42:00]

The CQ bit was checked by kjellander@webrtc.org

[commit-bot: I haz the power, 2015-11-09 19:42:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1437433002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1437433002/60001

[commit-bot: I haz the power, 2015-11-09 21:08:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-09 21:08:39]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[brucedawson, 2015-11-09 21:33:05]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2015-11-09 21:33:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1437433002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1437433002/60001

[commit-bot: I haz the power, 2015-11-09 23:33:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-09 23:33:33]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[kjellander_webrtc, 2015-11-10 06:51:29]

The CQ bit was checked by kjellander@webrtc.org

[commit-bot: I haz the power, 2015-11-10 06:51:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1437433002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1437433002/60001

[commit-bot: I haz the power, 2015-11-10 06:51:56]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-10 06:52:11]

Patchset 4 (id:??) landed as
https://crrev.com/952892a28a06a4ced120d4683d930699b9d730de
Cr-Commit-Position: refs/heads/master@{#10569}