Author Guido
Recipients Guido, georg.brandl, gvanrossum, serhiy.storchaka, vstinner
Date 2014-12-16.00:25:11
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1418689511.63.0.442689994987.issue23055@psf.upfronthosting.co.za>
In-reply-to
Content
Serhiy Storchaka: good call on changing my 'n += (width + precision) < 20 ? 20 : (width + precision);' into 'if (width < precision) width = precision;', I didn't realize that sprintf's space requirement entails using the largest of the two instead of adding the two together.

I noticed the apparently pointless width calculation in 'step 1' but decided not to touch it -- good that it's removed now though.

I will start doing more debugging based on this new patch now to ensure that the bug is gone now.

On a more design-related note, for the sake of readability and stability, I'd personally opt for implementing toned-down custom sprintf-like function that does exactly what it needs to do and nothing more, since a function like this one requires perfect alignment with the underlying sprintf() in terms of functionality, at the possible expense of stability and integrity issues like we see here. For instance, width and precision are currently overflowable, resulting in either a minus sign appearing in the resulant format string given to sprintf() (width and precision are signed integers), or completely overflowing it (ie. (uint64_t)18446744073709551617 == 1 ). Considering the latter example, how do we know sprintf uses the same logic?

Guido
History
Date User Action Args
2014-12-16 00:25:11Guidosetrecipients: + Guido, gvanrossum, georg.brandl, vstinner, serhiy.storchaka
2014-12-16 00:25:11Guidosetmessageid: <1418689511.63.0.442689994987.issue23055@psf.upfronthosting.co.za>
2014-12-16 00:25:11Guidolinkissue23055 messages
2014-12-16 00:25:11Guidocreate