| msg94601 - (view) |
Author: Hirokazu Yamamoto (ocean-city) *  |
Date: 2009-10-28 06:49 |
| Hello. There is following sentence in Modules/_io/bufferedio.c, PyErr_Format(PyExc_IOError, "Raw stream returned invalid position %" PY_PRIdOFF, (PY_OFF_T_COMPAT)n); and PY_PRIdOFF == "lld" when sizeof(off_t) == sizeof(long long). But it seems that PyErr_Format doesn't support lld as specifier. I noticed this because # define PY_OFF_T_COMPAT long long caused compile error on my good old VC6. ;-) (VC6 doesn't have it) |
|
|
| msg94602 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2009-10-28 06:51 |
| I believe r75728 and r75879 are related. |
|
|
| msg94604 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-10-28 07:12 |
| Thanks for reporting this. Do you know what the right conversion specifier is for print(f)ing something of long long type in MSVC? |
|
|
| msg94605 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-10-28 07:20 |
| The 'long long' define should have been PY_LONG_LONG. I don't know what the appropriate substitute for "%lld" is, though. |
|
|
| msg94606 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2009-10-28 07:21 |
| MSVC6 uses __int64 as 64bit integer, and printf uses "I64" as its specifier. |
|
|
| msg94607 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-10-28 07:31 |
| So PY_PRIdOFF should be "I64d"? Or just "I64"? |
|
|
| msg94608 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2009-10-28 07:37 |
| Oh, I was late. I agree with . printf("%I64d\n", 1I64 << 40); /* 1099511627776 */ So if PyErr_Format (actually, PyString_FromFormatV) will support PY_LONG_LONG, I think we can use same technique as PY_FORMAT_SIZE_T like #define PY_FORMAT_LONG_LONG "I64" /* On Windows */ #define PY_FORMAT_LONG_LONG "ll" /* On Unix */ |
|
|
| msg94609 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2009-10-28 07:43 |
| I was late again...? Hmm, I thought Python tracker told me that somebody else modified this issue. Anyway, printf can use both "%I64" and "%I64d" for signed 64bit integer, but should use "%I64u" for unsigned 64bit integer AFAIK. But PyErr_Format actually calls PyString_FromFormatV, and it's not treating %lld. So probably we should modify PyString_FromFormatV. |
|
|
| msg94610 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-10-28 07:48 |
| Thanks. I'm just going to fix Modules/io/_iomodule.h for now. But I agree that it might make sense to have a PY_FORMAT_OFF_T or PY_FORMAT_LONG_LONG in pyport.h, especially if uses of off_t become more widespread in the codebase. I also notice there are some uses of "%zd" in Modules/io that should be replaced with PY_FORMAT_SIZE_T. |
|
|
| msg94611 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-10-28 07:54 |
| Aargh. You're right, of course. PyString_FromFormatV needs to be updated, or avoided in this case. I'll look at this later today. |
|
|
| msg94612 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2009-10-28 07:59 |
| Sorry for confusion. I shouldn't have said last 3 lines in . :-( |
|
|
| msg94653 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-10-29 10:01 |
| I've rolled back all the changes I made trying to fix these format arguments; the rollback is in r75939 (trunk), r75941 (py3k) and r75942 (release31-maint). Next time I'll think a bit harder, get a code review, and make sure that the code gets tested on Windows before it goes in. I agree that what's needed here is %lld and %llu support in PyErr_Format and PyString_FromFormat(V); this seems like a useful thing to add in any case. ocean-city: you don't happen to have a patch available, do you? |
|
|
| msg94706 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2009-10-30 14:33 |
| No, I don't have it. |
|
|
| msg94756 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-10-31 14:07 |
| Patch to add %lld support to PyString_FromFormat(V). (Against the trunk.) |
|
|
| msg94760 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2009-10-31 16:35 |
| I tried your patch on windows, your patch worked great. One little thing: I think line 346 of patch can be wrapped with "#ifdef HAVE_LONG_LONG". |
|
|
| msg95302 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-11-15 17:06 |
| Added extra #ifdef HAVE_LONG_LONG as suggested. Committed to trunk in r76308. I'm working on the merge to py3k, which isn't entirely straightforward. |
|
|
| msg95305 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-11-15 19:14 |
| Just so I don't lose it, here's the current version of the forward merge of r76308 to py3k. In testing this, I found some other possible bugs in the py3k implementation of PyUnicode_FromFormat; I need to investigate and possibly fix those before this can be applied. |
|
|
| msg95352 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-11-16 17:02 |
| Merged to py3k in r76328. |
|
|
| msg95354 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-11-16 17:29 |
| ocean-city, now that "lld" and "llu" support has been added, could you retest this patch (offt_formats.patch) for me? |
|
|
| msg95664 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2009-11-24 07:28 |
| I think your patch is correct. (I couldn't check the behavior on error condition itself, because I wasn't sure how, but I checked test_io run on windows) |
|
|
| msg95699 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-11-24 20:59 |
| Thanks for testing! Fixed (again) in r76502 (trunk), r76503 (py3k). I don't think it's worth backporting the fix to the release branches, given that that would require also implementing %lld support in those branches. |
|
|