Author belopolsky
Recipients amaury.forgeotdarc, belopolsky, eric.smith, vstinner
Date 2011-01-06.21:30:57
SpamBayes Score 2.00007e-12
Marked as misclassified No
Message-id <AANLkTikw63aLzGyr7ZY2XHgSinz4EQETZ=k+viW0xN=N@mail.gmail.com>
In-reply-to <1294346956.83.0.829438107968.issue10833@psf.upfronthosting.co.za>
Content
On Thu, Jan 6, 2011 at 3:49 PM, STINNER Victor <report@bugs.python.org> wrote:
..
> eric> I always thought that one of the reasons for specifying the length
> eric> was in case a pointer pointed to garbage: at least you'd be limiting
> eric> how much trash was printed.
>
> No, it's because of the old (removed) 500 bytes limit.
>

How do you know?  I, for one, did use %.100s when formatting strings
long before I knew that there was a 500 bytes limit.

..
> belopolsky> Limiting field width when formatting error messages
> belopolsky> is a good safety measure.  It is not only the amount
> belopolsky> of garbage that can be spitted in error logs, if garbage
> belopolsky> is not null-terminated, ...
>
> Can you give me at least one example? I think that it is very unlikely, or just impossible.

I am concerned about conditions that are impossible in a valid
program.  However, if you have a buffer overflow that trashes your
tp_name pointer so that it suddenly resolves to a binary code section,
you will need all help you can get to find the bug.  Having the
program spit 100-character garbage in this case is often preferable to
a SEGV.

> But if I do replace "%.100s"  by "%s" in all the code base, I accept to check each
> time that the argument is null-terminated and/or not controlable by the user.

As long as we have ctypes, everything is controllable by user.  Again,
limiting field width when formatting strings is defensive programming.
 And defensive programming pays off when you have to chase bugs.

> And maybe keep "%.100s" if I am not sure.

Just keep "%.100s" always.  It does not hurt and it is less work for you. :-)

>
> belopolsky> Think of out of memory errors: ...
>
> PyErr_NoMemory() doesn't use PyErr_Format(), it's message is fixed, and the exception object is preallocated.

Indeed, there are no PyErr_Format(PyExc_MemoryError, ..) in the code
base, but there are a few PyErr_SetString(PyExc_MemoryError, ..) such
as PyErr_SetString(PyExc_MemoryError, "math.fsum partials") in
Modules/mathmodule.c.  Maybe these should be reviewed.  Still, there
may be code paths where OOM triggers an error other than MemoryError.
In any case, I don't see anything wrong with using fixed size buffers
in error handling and truncating too long output.

Code cleanup projects like this are rightfully frowned upon because
they are hard to review and usually error-prone.  However, if for each
change that you make, you would make sure that the error path is
covered by unit tests, it will definitely be an improvement.
History
Date User Action Args
2011-01-06 21:31:04belopolskysetrecipients: + belopolsky, amaury.forgeotdarc, vstinner, eric.smith
2011-01-06 21:30:57belopolskylinkissue10833 messages
2011-01-06 21:30:57belopolskycreate