REVIEW: PyArg_ParseTuple with "s" format and NUL: Bogus TypeError detail string.

Please review this, I'm worried that there are cases where convertitem() is returning a string that really should be overridden by the argument "help string". However, I'm worried that this change will get rid of useful messages (via the format "; help string"), when there otherwise wouldn't be. PyArg_ParseTuple when handling a string format "s", raises a TypeError when the passed string contains a NUL. However, the TypeError doesn't contain useful information. For example: syslog.syslog('hello\0there') TypeError: [priority,] message string This seems to be a thinko in Python/getargs.c at line 331: msg = convertitem(PyTuple_GET_ITEM(args, i), &format, p_va, flags, levels, msgbuf, sizeof(msgbuf), &freelist); if (msg) { seterror(i+1, msg, levels, fname, message); <<< Line 331 return cleanreturn(0, freelist); } This also applies to Python 3 trunk in line 390. I think that's supposed to be "msg" instead of "message" in the last argument. If I change it, I get:
I think it's safe to change "message" to "msg" because: "message" is the "help" string "[priority,] message string", but "msg" contains much more useful information. "msg" is in the "fall back if the last argument doesn't contain useful information" argument position, but "msg" is never NULL. "message" only is NULL if the format string doesn't contain ";". In every case I can find in the code, convertitem() is returning a much more useful string than the help string. Or perhaps it should do something like: if (msg) { seterror(i+1, msg, levels, fname, '%s (%s)' % ( msg, message )); Pardon my mixed C+Python, but you get the idea. Thoughts? Thanks, Sean -- [...] Premature optimization is the root of all evil. -- Donald Knuth Sean Reifschneider, Member of Technical Staff <jafo@tummy.com> tummy.com, ltd. - Linux Consulting since 1995: Ask me about High Availability

On Thu, Jul 23, 2009, Sean Reifschneider wrote:
Thoughts?
Please file a report at bugs.python.org to make sure it doesn't get lost. -- Aahz (aahz@pythoncraft.com) <*> http://www.pythoncraft.com/ "At Resolver we've found it useful to short-circuit any doubt and just refer to comments in code as 'lies'. :-)" --Michael Foord paraphrases Christian Muirhead on python-dev, 2009-03-22

"make test" in both python and py3k trunks were happy with this change, so I've documented the issue in Issue #6624 and committed it in 74277 (2.x) and 74278 (3.x). Sean -- "The only thing more expensive than hiring a professional is hiring an amateur." -- Red Adair, Oil Well Fire-Fighter Sean Reifschneider, Member of Technical Staff <jafo@tummy.com> tummy.com, ltd. - Linux Consulting since 1995: Ask me about High Availability

On Thu, Jul 23, 2009, Sean Reifschneider wrote:
Thoughts?
Please file a report at bugs.python.org to make sure it doesn't get lost. -- Aahz (aahz@pythoncraft.com) <*> http://www.pythoncraft.com/ "At Resolver we've found it useful to short-circuit any doubt and just refer to comments in code as 'lies'. :-)" --Michael Foord paraphrases Christian Muirhead on python-dev, 2009-03-22

"make test" in both python and py3k trunks were happy with this change, so I've documented the issue in Issue #6624 and committed it in 74277 (2.x) and 74278 (3.x). Sean -- "The only thing more expensive than hiring a professional is hiring an amateur." -- Red Adair, Oil Well Fire-Fighter Sean Reifschneider, Member of Technical Staff <jafo@tummy.com> tummy.com, ltd. - Linux Consulting since 1995: Ask me about High Availability
participants (2)
-
Aahz
-
Sean Reifschneider