From: SourceForge.net <no...@so...> - 2009-09-30 10:27:29
|
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 |