gnokii-users
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: gnokii-0.6.30 patch


From: Pawel Kot
Subject: Re: gnokii-0.6.30 patch
Date: Thu, 20 Oct 2011 09:29:04 +0200

Hi,

On Thu, Oct 20, 2011 at 08:59, Alexander V. Lukyanov <address@hidden> wrote:
> On Wed, Oct 19, 2011 at 02:40:01PM +0200, Pawel Kot wrote:
>> On Wed, Oct 19, 2011 at 14:22, Alexander V. Lukyanov <address@hidden> wrote:
>> > 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.
>>
>> The only one within smsd as far as I can tell.
>
> This is the real caller (gsm-statemachine.c:126). The function
> NK6510_IncomingSMS is part of incoming_functions. As you see,
> message_center is not allocated here.
>        /* Pass up the message to the correct phone function, with data if 
> necessary */
>        c = 0;
>        while (state->driver.incoming_functions[c].functions) {
>                if (state->driver.incoming_functions[c].message_type == 
> messagetype) {
>                        dprintf("Received message type %02x\n", messagetype);
>                        res = 
> state->driver.incoming_functions[c].functions(messagetype, message, 
> messagesize, data, state);
>                        temp = 0;
>                        break;
>                }
>                c++;
>        }

This is direct caller. The real caller is
gn_sm_functions(GN_OP_GetSMSCenter, data, state). The phone will not
respond with sms center information when not requested. It should be
allocated always before calling this function. And I believe it does.

>> > Yes, I've had problems with touching free'd memory. I tracked the bug using
>> > ElectricFence. The static structure was reused without proper clearing.
>>
>> Well, what I see is that the full structure is always zeroed before
>> reusing.
>
> It is reused in this loop (mysql.c:384):
>    do
>    {
>      error = WriteSMS (&sms);
>      sleep (1);
>    }
>    while ((error == GN_ERR_TIMEOUT || error == GN_ERR_FAILED) && numError++ < 
> 3);

Yes, but the code looks like:
  while ((row = mysql_fetch_row (res1)))
  {
    gn_sms sms;

    [...]
    gn_sms_default_submit (&sms);    [*]
    [...]
    do
    {
      error = WriteSMS (&sms);
      sleep (1);
    }
    while ((error == GN_ERR_TIMEOUT || error == GN_ERR_FAILED) &&
numError++ < 3);
    [...]
  }

in [*] it is cleared. However it might be possible that something is
going badly in the retries loop. I'll have a closer look.

take care,
-- 
Pawel Kot



reply via email to

[Prev in Thread] Current Thread [Next in Thread]