%lld for PyErr_Format (Modules/_io/bufferedio.c)

[ocean-city]

BPO	7228
Nosy	@mdickinson
Files	add_lld_format.patch add_lld_format_py3k.patch offt_formats.patch: Fix format mismatch when printing off_t type.

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/mdickinson'
closed_at = <Date 2009-11-24.20:59:24.831>
created_at = <Date 2009-10-28.06:49:40.111>
labels = ['type-feature']
title = '%lld for PyErr_Format (Modules/_io/bufferedio.c)'
updated_at = <Date 2009-11-24.20:59:24.830>
user = 'https://bugs.python.org/ocean-city'

bugs.python.org fields:

activity = <Date 2009-11-24.20:59:24.830>
actor = 'mark.dickinson'
assignee = 'mark.dickinson'
closed = True
closed_date = <Date 2009-11-24.20:59:24.831>
closer = 'mark.dickinson'
components = []
creation = <Date 2009-10-28.06:49:40.111>
creator = 'ocean-city'
dependencies = []
files = ['15240', '15344', '15346']
hgrepos = []
issue_num = 7228
keywords = ['patch']
message_count = 21.0
messages = ['94601', '94602', '94604', '94605', '94606', '94607', '94608', '94609', '94610', '94611', '94612', '94653', '94706', '94756', '94760', '95302', '95305', '95352', '95354', '95664', '95699']
nosy_count = 2.0
nosy_names = ['mark.dickinson', 'ocean-city']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue7228'
versions = ['Python 2.7']

[ocean-city]

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)

[ocean-city]

I believe r75728 and r75879 are related.

[mdickinson]

Thanks for reporting this.

Do you know what the right conversion specifier is for print(f)ing
something of long long type in MSVC?

[mdickinson]

The 'long long' define should have been PY_LONG_LONG. I don't know what
the appropriate substitute for "%lld" is, though.

[ocean-city]

MSVC6 uses __int64 as 64bit integer, and printf uses "I64" as its specifier.

[mdickinson]

So PY_PRIdOFF should be "I64d"? Or just "I64"?

[ocean-city]

Oh, I was late. I agree with msg94605.

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 */

[ocean-city]

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.

[mdickinson]

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.

[mdickinson]

Aargh. You're right, of course. PyString_FromFormatV needs to be
updated, or avoided in this case.

I'll look at this later today.

[ocean-city]

Sorry for confusion. I shouldn't have said last 3 lines in msg94601. :-(

[mdickinson]

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?

[ocean-city]

No, I don't have it.

[mdickinson]

Patch to add %lld support to PyString_FromFormat(V). (Against the trunk.)

[ocean-city]

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

[mdickinson]

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.

[mdickinson]

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.

[mdickinson]

Merged to py3k in r76328.

[mdickinson]

ocean-city, now that "lld" and "llu" support has been added, could you
retest this patch (offt_formats.patch) for me?

[ocean-city]

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)

[mdickinson]

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.