[Python-Dev] 2.4 vs Windows vs bsddb

[Tim Peters]

[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/db_err.c db_strerror() 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.