Menu
▾
▴
#424 Match the %p format in MSVCRT
closed-fixed
2014-07-11
2009-08-25
No
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 <peda@lysator.liu.se>
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.
Match the %p format in MSVCRT
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().
Match the %p format in MSVCRT, now with zero fill.
Oops, forgot to zero fill...
Fixed in printf-pointer-2.patch
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.
> 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.
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.
Patch, as applied.
Yes, looks good. Thanks!
(But to me it seems rather arbitrary to call the Microsoft *scanf functions buggy. After all, their only requirement for %p is to handle whatever the *printf functions spew out. They're silly, but nut buggy IMHO)
Cheers,
Peter
Well, I'm not prepared to argue the point, but any routine intended to parse hexadecimal data, and which isn't equipped to handle the standard radix indicating prefix for such data, is just plain broken, IMO.