Re: svn commit: r1751207 - in /subversion/branches/1.9.x: ./ STATUS subversion/libsvn_subr/win32_crashrpt.c

Bert Huijben <bert_at_qqmail.nl> writes:

>> #if defined(_M_IX86)
>> -#define FORMAT_PTR "0x%08x"
>> +#define FORMAT_PTR "0x%08Ix"
>> #elif defined(_M_X64)
>> -#define FORMAT_PTR "0x%016I64x"
>> +#define FORMAT_PTR "0x%016Ix"
>> #endif
>
> This changes the Visual C++ 64 bit specifier in the long specifier.
> On Visual C++ longs are 32 bit when compiling for AMD64. You would need
> long long in newer VC compilers or the explicit 64 bit support to properly
> handle this, as was done before the patch
>
> I would say this introduces *more* unspecified behavior.

(Sorry that it took me a while to reply.)

I don't quite see where the behavior we have now is undefined. The values
that we pass to a FORMAT_PTR are either UINT_PTR's or DWORD_PTR's.
Both of them are unsigned 32-bit or 64-bit types, depending on whether we
compile for 32 or for 64 bits. For instance, a DWORD_PTR is defined as
follows, per the spec [1, 2]:

  typedef unsigned __int3264 ULONG_PTR;

  typedef ULONG_PTR DWORD_PTR;

With that in mind, an "%Ix" format specifier matches the type of what we
pass in varargs, and this behavior is defined.

Or am I missing something?

> But when we use the system sprintf(), fprintf(), anyway... why not just
> use "0x%p" for all calling conventions?

Sounds like a nice improvement that we can probably do in trunk.

[1] https://msdn.microsoft.com/en-us/library/cc230322
[2] https://msdn.microsoft.com/en-us/library/cc230394

Regards,
Evgeny Kotkov
Received on 2016-07-07 19:02:11 CEST