Re: AT2 with wavecom again

On Sun, Mar 17, 2002 at 11:37:24AM +0200, Oded Arbel wrote:
> There's a bug in smsc_at2_create in line 1054 where octstr_destroy() is
> called, but without setting the Octstr* destoryed to NULL
> (octstr_destroy() does not do that). a simple fix would be to change all
> octstr_destroy calls to O_DESTROY macro instead (which does checking and
> also assigns NULL to the destroyed pointer).

Note that the checking is not needed, octstr_destroy(NULL) is guaranteed
to do nothing.  This is precisely to avoid the need of checking every
call.

Actually there seems to be a much bigger bug in the piece of code in
question:

    if(modem_type_string != NULL) {
        if(octstr_compare(modem_type_string, octstr_imm("auto")) ||
           octstr_compare(modem_type_string, octstr_imm("autodetect")))
            octstr_destroy(modem_type_string);
    }

This uses boolean logic on the result of octstr_compare().  But
octstr_compare returns -1, 0, or 1, based on the result of the comparison.
This means that if you use it as a boolean directly, then equality is
"false" and inequality is "true".  So the condition in this code
fragment is always true -- modem_type_string can't be equal to _both_
of the strings.

If you're using octstr_compare as an equality test, then I recommend
always explicitly comparing with 0 so that it's clear what you mean.

Richard Braakman