Bugs item #1274069, was opened at 2005-08-26 16:25 Message generated for change (Comment added) made by birkenfeld You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1274069&group_id=5470
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: Extension Modules Group: Python 2.5 Status: Open Resolution: None Priority: 5 Submitted By: Tim Peters (tim_one) Assigned to: Nobody/Anonymous (nobody) Summary: bz2module.c compiler warning Initial Comment: On Python HEAD, there's a new warning while compiling bz2module.c (MSVC 7.1): python\Modules\bz2module.c(1095) : warning C4244: '=' : conversion from 'Py_off_t' to 'int', possible loss of data Looks like a legitimate complaint to me. The line in question: readsize = offset-bytesread; Those have types int, Py_off_t, and int respectively. Is it true that offset-bytesread _necessarily_ fits in an int? If so, a comment should explain why, and a cast should be added to squash the warning. Or If the difference doesn't necessarily fit in an int, the code is wrong. ---------------------------------------------------------------------- >Comment By: Reinhold Birkenfeld (birkenfeld) Date: 2005-08-27 16:36 Message: Logged In: YES user_id=1188172 Okay, next try. Someone with Windows should check this before I check in. ---------------------------------------------------------------------- Comment By: Martin v. Löwis (loewis) Date: 2005-08-27 14:55 Message: Logged In: YES user_id=21627 As Tim says: the cast deserves a comment, explaining why it cannot overflow. Even when readsize is size_t, it might still overflow, since Py_off_t might be wider than size_t. Apart from that, the patch looks good - but I can't check whether it silences the warning on Windows. ---------------------------------------------------------------------- Comment By: Reinhold Birkenfeld (birkenfeld) Date: 2005-08-27 14:27 Message: Logged In: YES user_id=1188172 Attaching patch for f->pos and f->size as Py_off_t. ---------------------------------------------------------------------- Comment By: Martin v. Löwis (loewis) Date: 2005-08-27 12:30 Message: Logged In: YES user_id=21627 Clearly, the difference is bound by buffersize (since, if it is larger, readsize is set to buffersize). buffersize is initialized to SMALLCHUNK, which is 8192, and never changed, so yes, the difference fits to an int an all supported platforms. OTOH, the condition of the if statement is bogus: if ((size_t)offset-bytesread > buffersize) offset is of type Py_off_t, which is a 64-bit type on most platforms. Casting it to size_t causes truncation on 32-bit platforms. Furthermore, I think self->pos should be of type Py_off_t, as it will wrap around for files larger >2GB on 32-bit platforms; the same for self->size. ---------------------------------------------------------------------- Comment By: Reinhold Birkenfeld (birkenfeld) Date: 2005-08-26 17:05 Message: Logged In: YES user_id=1188172 I think declaring readsize as size_t should fix the problem. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1274069&group_id=5470 _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com