Don't pass a string16 to %s.

Don't pass a string16 to %s.

clang warns:
..\..\win8\delegate_execute\delegate_execute.cc(97,39) :  error(clang): cannot pass object of non-trivial type 'const base::string16' (aka 'const basic_string<wchar_t, char_traits<wchar_t>, allocator<wchar_t> >') through variadic function; call will abort at runtime [-Wnon-pod-varargs]
           operation.mutex().c_str(), operation.relaunch_flags());
                                      ^
1 error generated.

How did this every work? My guess is that %s interpreted the size_
member variable at the start of string16 as C string, and since the size
usually has a at least the upper byte set to 0, this ended up logging
1-2 garbage bytes.

BUG=82385
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281750

[Nico, 2014-07-06 20:07:24]

Since noone seems to look at the output of this line, maybe we can just delete
it instead?

[Nico, 2014-07-08 02:57:15]

+grt, Robert is apparently away.
On Jul 6, 2014 1:07 PM, <thakis@chromium.org> wrote:

> Reviewers: robertshield,
>
> Message:
> Since noone seems to look at the output of this line, maybe we can just
> delete
> it instead?
>
> Description:
> Don't pass a string16 to %s.
>
> clang warns:
> ..\..\win8\delegate_execute\delegate_execute.cc(97,39) :  error(clang):
> cannot
> pass object of non-trivial type 'const base::string16' (aka 'const
> basic_string<wchar_t, char_traits<wchar_t>, allocator<wchar_t> >') through
> variadic function; call will abort at runtime [-Wnon-pod-varargs]
>            operation.mutex().c_str(), operation.relaunch_flags());
>                                       ^
> 1 error generated.
>
> How did this every work? My guess is that %s interpreted the size_
> member variable at the start of string16 as C string, and since the size
> usually has a at least the upper byte set to 0, this ended up logging
> 1-2 garbage bytes.
>
> BUG=82385
>
> Please review this at https://codereview.chromium.org/370943002/
>
> SVN Base: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+2, -2 lines):
>   M win8/delegate_execute/delegate_execute.cc
>
>
> Index: win8/delegate_execute/delegate_execute.cc
> diff --git a/win8/delegate_execute/delegate_execute.cc
> b/win8/delegate_execute/delegate_execute.cc
> index 837505b7c423287f767762f7aebe40285d72d766..
> 1c4e9ec76742c2e80c3eab5c354a64b9a8163f15 100644
> --- a/win8/delegate_execute/delegate_execute.cc
> +++ b/win8/delegate_execute/delegate_execute.cc
> @@ -93,8 +93,8 @@ using delegate_execute::DelegateExecuteOperation;
>  using base::win::ScopedHandle;
>
>  int RelaunchChrome(const DelegateExecuteOperation& operation) {
> -  AtlTrace("Relaunching [%ls] with flags [%s]\n",
> -           operation.mutex().c_str(), operation.relaunch_flags());
> +  AtlTrace("Relaunching [%ls] with flags [%ls]\n",
> +           operation.mutex().c_str(), operation.relaunch_flags().c_
> str());
>    ScopedHandle mutex(OpenMutexW(SYNCHRONIZE, FALSE,
> operation.mutex().c_str()));
>    if (mutex.IsValid()) {
>      const int kWaitSeconds = 5;
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2014-07-08 04:35:50]

(actually +grt)

[grt (UTC plus 2), 2014-07-08 14:10:22]

doh! lgtm.

[grt (UTC plus 2), 2014-07-08 14:12:12]

On 2014/07/06 20:07:24, Nico (away) wrote:
> Since noone seems to look at the output of this line, maybe we can just delete
> it instead?

No one may be looking at that line now, but having it there may prove useful in
diagnosing failures in the future. It's valuable to see what parts of the code
are hit during the crazy launch/relaunch dance(s). Thanks.

[Nico, 2014-07-08 14:44:05]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2014-07-08 14:45:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thakis@chromium.org/370943002/1

[commit-bot: I haz the power, 2014-07-08 14:48:03]

Change committed as 281750