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
