Re: gnokii-0.6.30 patch

[Alexander V. Lukyanov]

On Wed, Oct 19, 2011 at 12:22:55PM +0200, Pawel Kot wrote:
> On Wed, Oct 19, 2011 at 11:29, Alexander V. Lukyanov <address@hidden> wrote:
> > On Wed, Oct 19, 2011 at 10:28:51AM +0200, Pawel Kot wrote:
> >> --- gnokii-0.6.30/common/phones/nk6510.c      2011-01-23 
> >> 17:27:32.000000000 +0300
> >> +++ gnokii-0.6.30+/common/phones/nk6510.c     2011-03-11 
> >> 14:02:25.000000000 +0300
> >> +             if(!data->message_center)
> >> +                     data->message_center = calloc(1, 
> >> sizeof(data->message_center[0]));
> >>
> >> No, I want to avoid it. If (!data->message_center) it should exit with
> >> error in this place. It is a responsibility of the caller to allocate
> >> it.
> >
> > Ok, then the caller should be fixed.
> 
> Caller looks good to me:
>       dt->message_center = calloc (1, sizeof (gn_sms_message_center));
>       dt->message_center->id = 1;
>       if ((status = gn_sm_functions (GN_OP_GetSMSCenter, dt, sm)) ==
> GN_ERR_NONE)

I don't think it is the only caller of that function.

> Do you have the backctrace and log when it fails?

Unfortunately, no. I debugged it half a year ago. gnokii-smsd failed on
some incoming SMS'es by NULL dereference.

> >>      free (dt->message_center);
> >> +    dt->message_center = 0;
> >>
> >> I'd prefer NULL instead of 0. But that should not matter actually. We
> >> allocate the structure unconditionally.

I think it is good to zero free'd pointers to avoid subtle problems.

> >>
> >>    free (dt->sms->reference);
> >> +  dt->sms->reference=0;
> >> +  dt->sms->parts=0;
> >>    free (dt);
> >>
> >> Does not make sense as we free() it right after.
> >
> > It's not quite so. dt->sms is not free'd.
> 
> Right. Because it is static. And no need to zero the parts then. Did
> you have any problems because of that?

Yes, I've had problems with touching free'd memory. I tracked the bug using
ElectricFence. The static structure was reused without proper clearing.

I use gnokii-smsd for sending SMS from mysql DB (1000-1500 per month) and
sometimes I get incoming SMS. Before the patching it failed regularly.

-- 
   Alexander.