[Python-Dev] remaining issues from Klocwork static analysis (original) (raw)

"Martin v. Löwis" martin at v.loewis.de
Wed Jul 26 07:05:44 CEST 2006

Neal Norwitz wrote:

Not quite sure what it is complaining about, but

else if (PyTupleCheck(closure)) { PyXINCREF(closure); } looks indeed suspicious: Why do we check for NULL (XINCREF) when we know closure can't be NULL (TupleCheck). Drop the X, and see if the warning goes away Yes, I definitely think dropping the X would make the warning go away. Do we want to check for a NULL pointer and raise an exception? The docs don't address the issue, so I think if we added a check, ie: if (closure && PyTupleCheck(closure)) and got rid of the X that would be fine as well.

The docs do address the issue:

\var{closure} must be \var{Py_None} or a tuple of cell objects.

It doesn't allow for NULL, and None indicates that the closure should become NULL. The only caller of it in the core will never pass NULL.

If you want to check that this is not NULL on the grounds that somebody may call it incorrectly, then you should also check that op is not NULL, because somebody may call it incorrectly.

The first line of data is (fro unicodedatadb.h) is:

static unsigned int decompdata[] = { 0, 257, 32, 514, 32, 776, 259, 97, 514, 32, 772, 259, 50, 259, 51, 514,

Read this as 0: sentinel 257 = 256 | 1: length 1, prefix 1 32: U+0020 514 = 512 | 2: length 2, prefix 2 32: U+0020 776: U+308 ...

Is that correct? If so, would it be correct to add:

unsigned short prefixindex = decompdata[index] & 255; assert(prefixindex < (sizeof(decompprefix)/sizeof(*decompprefix)));

Yes, that would be correct.

Regards, Martin

More information about the Python-Dev mailing list