Re: [BUG] Smsbox crash with empty messages

----- Original Message -----
From: "Aarno Syvänen" <[email protected]>
Cc: "Kannel-devel (E-mail)" <[email protected]>
Sent: Monday, March 04, 2002 3:53 PM
Subject: Re: [BUG] Smsbox crash with empty messages


> Bruno David Rodrigues wrote:
> >
> > ----- Original Message -----
> > From: "Stipe Tolj" <[email protected]>
> > To: "Bruno David Rodrigues" <[email protected]>
> > Cc: <[email protected]>
> > Sent: Monday, March 04, 2002 12:49 PM
> > Subject: Re: [BUG] Smsbox crash with empty messages
> >
> > > Bruno David Rodrigues wrote:
> > > >
> > > > smsbox panics on smsbox line 1180 because of a empty text field
(NULL):
> > > >
> > > > http://www.kannel.3glab.org/cgi-bin/viewcvs.cgi/gateway/gw/smsbox.c?
> > > > annotate=1.174#1180
> > > >
> > > > In patch 1.164 at 2002/01/25:
> > > >  udh == NULL ? ( text == NULL ? "" : octstr_get_cstr(text) ) : "<<
UDH
> > >>");
> > > >
> > > >  udh == NULL ? ( text == NULL ? "" : octstr_get_cstr(text) ) :
> > > >                 octstr_get_cstr(text));
> > > >
> > > > Why ?!
> > > > I'm reverting this line
> > >
> > > octstr_get_cstr() requires an non-NULL octstr pointer.
> > >
> > > So if text == NULL then octstr_get_cstr() throws the assertion error
> > > and panics the smsbox.
> > >
> > > BTW, in the second statement you are not garanteeing that call to
> > > octstr_get_cstr() does not have text == NULL, it's only handled in the
> > > case udh == NULL.
> >
> > :P
> >
> > I asked why did syvanen changed the code like that !
> >
> > I know what octstr_get_cstr does ;), that's why I've looked everywhere
for
> > lines using it and tried to be careful and inserted the (text == NULL ?
"" :
> > octstr_get_cstr(text)) back when I added the possibility of empty
messages
> > to smsbox.
> >
> > And it this particular line, we've talked about that, in general, when
you
> > have
> > a udh, the text probably is binary (there weren't EMS back then), so it
> > was not necessary to print the text when exists a udh. I guess syvanen
> > thought in printing the text even if there is a udh.
>
> Actually syvanen did not think *anything* :(( You should *not* commit
> any-
> thing like that. And I think that we should add empty message tests to
> all
> test scripts. Make check is supposed to catch errors like this.
>
> aarno

What should I NOT commit ?
I've changed the line to this:

    info(0, "sendsms sender:<%s:%s> (%s) to:<%s> msg:<%s>",
         octstr_get_cstr(urltrans_username(t)),
         octstr_get_cstr(newfrom),
         octstr_get_cstr(client_ip),
         octstr_get_cstr(to),
         ( text == NULL ? "" : octstr_get_cstr(text) ));

I'm sorry, I'm not acusing you, but it's frustrating to have been working
for some days
looking for every piece of code that could crash with empty messages, and
now,
at least two month later, the bug reappears.