Bruno David Rodrigues wrote: > > ----- 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.
Sorry being misleading. *You* can commit your proposal, it was *I* that should not have committed the bug you kindly corrected. You can even accuse me for doing this kind of thing. And yes, I will add a test for empty messages, and NULL messages to check_sendsms script. aarno
