[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Bert Huijben <bert_at_qqmail.nl>
Date: Mon, 4 Jul 2016 11:01:49 +0200

> -----Original Message-----
> From: svn-role_at_apache.org [mailto:svn-role_at_apache.org]
> Sent: maandag 4 juli 2016 06:01
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1751207 - in /subversion/branches/1.9.x: ./ STATUS
> subversion/libsvn_subr/win32_crashrpt.c
>
> Author: svn-role
> Date: Mon Jul 4 04:00:50 2016
> New Revision: 1751207
>
> URL: http://svn.apache.org/viewvc?rev=1751207&view=rev
> Log:
> Merge the r1663253 group from trunk:
>
> * r1663253, r1704821, r1738659, r1738828
> Fix a few instances of undefined behavior in Win32 crash reporter.
> Justification:
> Potentially crashing in a crash reporter is bad.
> Votes:
> +1: kotkov, ivan, jamessan
>
> Modified:
> subversion/branches/1.9.x/ (props changed)
> subversion/branches/1.9.x/STATUS
> subversion/branches/1.9.x/subversion/libsvn_subr/win32_crashrpt.c

 
> Modified: subversion/branches/1.9.x/subversion/libsvn_subr/win32_crashrpt.c
> URL:
> http://svn.apache.org/viewvc/subversion/branches/1.9.x/subversion/libsvn_sub
> r/win32_crashrpt.c?rev=1751207&r1=1751206&r2=1751207&view=diff
> ================================================================
> ==============
> --- subversion/branches/1.9.x/subversion/libsvn_subr/win32_crashrpt.c
> (original)
> +++ subversion/branches/1.9.x/subversion/libsvn_subr/win32_crashrpt.c Mon
> Jul 4 04:00:50 2016
> @@ -53,9 +53,9 @@ HANDLE dbghelp_dll = INVALID_HANDLE_VALU
> #define LOGFILE_PREFIX "svn-crash-log"
>
> #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.

In practice 64 bit is reserved for each integer type provided in varargs in the Windows AMD64 calling convention, so it doesn't change that much and the runtime result will be +- identical....

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

(Rest of the patch looks ok)

        Bert
Received on 2016-07-04 11:02:08 CEST

This is an archived mail posted to the Subversion Dev mailing list.