Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#424 Match the %p format in MSVCRT

closed-fixed
2014-07-11
2009-08-25
Peter Rosin
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.

Discussion

  • Peter Rosin
    Peter Rosin
    2009-08-25

    Match the %p format in MSVCRT

     
    Attachments
  • Keith Marshall
    Keith Marshall
    2009-08-25

    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().

     
  • Keith Marshall
    Keith Marshall
    2009-08-25

    • milestone: --> 519165
    • assigned_to: nobody --> keithmarshall
     
  • Peter Rosin
    Peter Rosin
    2009-08-26

    Match the %p format in MSVCRT, now with zero fill.

     
  • Peter Rosin
    Peter Rosin
    2009-08-26

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

     
  • Peter Rosin
    Peter Rosin
    2009-09-23

    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.

     
  • Keith Marshall
    Keith Marshall
    2009-09-23

    > 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.

     
  • Keith Marshall
    Keith Marshall
    2009-09-30

    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.

     
  • Keith Marshall
    Keith Marshall
    2009-09-30

    • milestone: 519165 --> IINR_-_Include_In_Next_Release
    • status: open --> pending-fixed
     
  • Keith Marshall
    Keith Marshall
    2009-09-30

    Patch, as applied.

     
    Attachments
  • Peter Rosin
    Peter Rosin
    2009-09-30

    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

     
  • Peter Rosin
    Peter Rosin
    2009-09-30

    • status: pending-fixed --> open-fixed
     
  • Keith Marshall
    Keith Marshall
    2009-10-02

    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.

     
  • Keith Marshall
    Keith Marshall
    2009-10-02

    • status: open-fixed --> closed-fixed