[Python-Dev] remaining issues from Klocwork static analysis

[Michael Hudson]

"Neal Norwitz" <nnorwitz at gmail.com> writes:

> On 7/25/06, "Martin v. Löwis" <martin at v.loewis.de> wrote:
>> >
>> > 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 && PyTuple_Check(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.
>
> We never really did address this issue did?  A while back we talked
> about whether to assert vs check and do PyErr_BadInternalCall().  I
> don't remember a clear resolution (though my memory).  I vaguely
> remember a preference towards asserting, but I don't know if that was
> in all cases or maybe it was just my preference. :-)
>
> I'm happy to assert here too.  But it's really a broader question.  I
> guess I'm even happy to just remove the X.  It would be nice to handle
> this consistently going forward.

I think I'm rather in favour of assert()ing this sort of thing.  If
you're programming in C, you can cause crashes any which way and
removing one doesn't seem worth making correct usage pay any kind of
(admittedly miniscule) performance penalty.  It would be nice if API
docs explicitly stated which pointer arguments could be NULL, and then
it would be a programming error to pass a NULL pointer argument in any
other place.  I have no idea how far away from this we are already :-)

Cheers,
mwh

-- 
  Gullible editorial staff continues to post links to any and all
  articles that vaguely criticize Linux in any way.
         -- Reason #4 for quitting slashdot today, from
            http://www.cs.washington.edu/homes/klee/misc/slashdot.html