[Mingw-notify] [ mingw-Patches-2844514 ] Match the %p format in MSVCRT

[SourceForge.net <no...@so...>]

Patches item #2844514, was opened at 2009-08-25 20:37
Message generated for change (Comment added) made by keithmarshall
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=302435&aid=2844514&group_id=2435

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: runtime
>Group: IINR - Include In Next Release
>Status: Pending
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Peter Rosin (pekberg)
Assigned to: Keith Marshall (keithmarshall)
Summary: Match the %p format in MSVCRT

Initial Comment:
As I wrote on the mingw-users list, this program produces bad output:

$ cat scan.c
#include <stdio.h>
int main(void)
{
     void *p;
     char buf[100];

     snprintf(buf, sizeof(buf), "%p", &p);
     printf("%s\n", buf);
     sscanf(buf, "%p", &p);
     printf("%p\n", p);

     return 0;
}
$ gcc -o scan scan.c
$ ./scan
0x22ff6c
00000000

I expect the below output, which is what I get with _snprintf instead of snprintf
0022FF6C
0022FF6C

Here's an untested patch.

2009-08-25  Peter Rosin  <pe...@ly...>

	Match the %p format in MSVCRT.

	* mingwex/stdio/pformat.c (__pformat) [%p]: The MSVCRT scanf family
	expects %X output from %p, so match that here.


----------------------------------------------------------------------

>Comment By: Keith Marshall (keithmarshall)
Date: 2009-09-30 10:27

Message:
Okay.  I've now had a chance to review your patch, thanks.  Unfortunately,
it introduces an undesirable side effect -- it makes it impossible for
client code to specify its own minimum field width (precision) for %p
format.  (Neither does it allow suppression of the "zero padding at left"
attribute, much as my original implementation didn't allow suppression of
the "0x" prefix attribute -- the original cause of your problem).

I've applied the alternative patch, as attached; I believe this should
resolve your issue.

----------------------------------------------------------------------

Comment By: Keith Marshall (keithmarshall)
Date: 2009-09-23 10:55

Message:
> I'm baffled that you are trying to portray this as
> an application responsibility.

I'm not; I'm merely pointing out that applications for MS-Windows don't
have an automatic right to assume that snprintf() is available, because
Microsoft don't provide it.  That our implementation produces output in a
format which Microsoft's _sscanf() cannot read, (and this is definitively a
bug in Microsoft's implementation) is an unfortunate feature, which
ideally, we should work around.

> I do not know how to best fix the problem...

Perhaps your autoconf script could check for _snprintf() before
snprintf(), and use the former for preference?  Or simply a `#ifdef _WIN32'
selection within your source code, to substitute _snprintf(), (which should
always be available if _WIN32 is defined).

> but I just can't see this as an application bug.

No.  Fundamentally, it's Microsoft's broken sscanf() implementation which
is at fault, but since we can't readily get them to fix that, we need to
implement a workaround in libmingwex.a.  AIUI, the alternative
implementation of snprintf(), in libmingwex.a, arose out of a need, within
GCC itself, for a C99 conforming implementation, (which Microsoft *still*
do not provide).  I'm not rejecting your patch; it does provide an
effective solution.  However, since I'm not fully conversant with the GCC
requirement, I'm being cautious about changing it, without the blessing of
those who I expect to know better than I, i.e. Danny or Aaron.  That said,
they have had ample time to comment; if they don't break it before the end
of this week, I shall assume that their deafening silence implies tacit
approval, and shall commit anyway.

----------------------------------------------------------------------

Comment By: Peter Rosin (pekberg)
Date: 2009-09-23 09:18

Message:
Keith Marshall wrote:
> Also note that, to maintain guaranteed consistency of
> implementation, your own application should be pairing
> _snprintf(), and not snprintf(), with Microsoft's sscanf().

And that's just the problem. My application did not
ask for this mismatch, the mismatch is in the defaults.
I have simple autoconf tests that checks if snprintf is
there. It is. (it assumes sscanf is available). Dandy,
so use it. Then shit hits the fan and I'm in trouble
when they are not compatible. How should I know
they are not compatible? I didn't say I wanted
implementations from different libraries. It was not
*me* that mixed up mingwex and msvcrt. It was
whoever added *both* -lmoldname and -lmingwex
to the defaults, when they don't mix and match.

I'm baffled that you are trying to portray this as an
application responsibility.

I do not know how to best fix the problem, but
I just can't see this as an application bug.

----------------------------------------------------------------------

Comment By: Peter Rosin (pekberg)
Date: 2009-08-26 05:03

Message:
Oops, forgot to zero fill...
Fixed in printf-pointer-2.patch

----------------------------------------------------------------------

Comment By: Keith Marshall (keithmarshall)
Date: 2009-08-25 21:40

Message:
Yep.  That's more or less the change I had in mind -- I might also set
stream.precision, to preserve complete field width compatibility with the
MS implementation.  However, as I mentioned in my follow up to your list
posting, I won't progress it until I have confirmation from Aaron or Danny,
that it won't break GCC itself.

Please note that the change from lower to upper case formatting isn't
strictly necessary -- Microsoft's sscanf() will happily accept either case,
but it does object to the radix prefix, in either case.

Also note that, to maintain guaranteed consistency of implementation, your
own application should be pairing _snprintf(), and not snprintf(), with
Microsoft's sscanf().

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=302435&aid=2844514&group_id=2435