[Python-Dev] 2.4 vs Windows vs bsddb (original) (raw)

Tim Peters tim.peters at gmail.com
Tue Oct 10 21:35:11 CEST 2006

[Gregory P. Smith]

It seems bad form to C assert() within a python extension. crashing is bad. Just code it to not copy the string in that case. The exception type should convey enough info alone and if someone actually looks at the string description of the exception they're welcome to notice that its missing info and file a bug (it won't happen; the strings come from the BerkeleyDB or C library itself).

The proper use of C's assert() in Python (whether core or extension) is to strongly document a condition the author believes /must/ be true. It's a strong sanity-check on the programmer's beliefs about necessary invariants, things that must be true under all possible conditions. For example, it would always be wrong to assert that the result of calling malloc() with a non-zero argument is non-NULL; it would be correct (although trivially and unhelpfully so) to assert that the result is NULL or is not NULL.

Given that, the assert() in question looks fine to me:

    if (_db_errmsg[0] && bytes_left < (sizeof(errTxt) - 4)) {
        bytes_left = sizeof(errTxt) - bytes_left - 4 - 1;
        assert(bytes_left >= 0);

We can't get into the block unless

bytes_left < sizeof(errTxt) - 4

is true. Subtracting bytes_left from both sides, then swapping LHS and RHS:

sizeof(errTxt) - bytes_left - 4 > 0

which implies

sizeof(errTxt) - bytes_left - 4 >= 1

Subtracting 1 from both sides:

sizeof(errTxt) - bytes_left - 4 - 1 >= 0

And since the LHS of that is the new value of bytes_left, it must be true that

 bytes_left >= 0

Either that, or the original author (and me, just above) made an error in analyzing what must be true at this point. From

bytes_left < sizeof(errTxt) - 4

it's not /instantly/ obvious that

bytes_left >= 0

inside the block, so there's value in assert'ing that it's true. It's both documentation and an executable sanity check.

In any case, assert() statements are thrown away in a release build, so can't be a cause of abnormal termination then.

As for the strncat instead of strcat that is good practice. The buffer is way more than large enough for any of the error messages defined in the berkeleydb common/dberr.c dbstrerror() function but the C library could supply its own unreasonably long one in some unforseen circumstance.

That's fine -- there "shouldn't have been" a problem with using any standard C function here. It was just the funky linker step on Windows on the 2.4 branch that was hosed. Martin figured out how to repair it, and there's no longer any problem here. In fact, even the been-there-forever linker warnings in 2.4 on Windows have gone away now.

More information about the Python-Dev mailing list