[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: gnokii-0.6.30 patch
From: |
Alexander V. Lukyanov |
Subject: |
Re: gnokii-0.6.30 patch |
Date: |
Thu, 20 Oct 2011 10:59:35 +0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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++;
}
> >> Do you have the backctrace and log when it fails?
> >> >> 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.
>
> Fair enough. But I'd better see where freed pointers are reused
> without proper allocation...
It is easier to find by setting them to zero or some poison.
> > 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);
The same in file.c, pq.c, sqlite.c.
> Still, would advice using the git version. I'll submit a subset of
> your patch today.
I'll try the git version and report the results.
--
Alexander.
- Re: gnokii-0.6.30 patch, Pawel Kot, 2011/10/19
- Re: gnokii-0.6.30 patch, Alexander V. Lukyanov, 2011/10/20
- Re: gnokii-0.6.30 patch, Pawel Kot, 2011/10/19
- Re: gnokii-0.6.30 patch, Alexander V. Lukyanov, 2011/10/20
- Re: gnokii-0.6.30 patch, Pawel Kot, 2011/10/19
- Re: gnokii-0.6.30 patch,
Alexander V. Lukyanov <=
- Re: gnokii-0.6.30 patch, Pawel Kot, 2011/10/20
- Re: gnokii-0.6.30 patch, Alexander V. Lukyanov, 2011/10/20
- Re: gnokii-0.6.30 patch, Pawel Kot, 2011/10/20
- Re: gnokii-0.6.30 patch, Alexander V. Lukyanov, 2011/10/20
- Re: gnokii-0.6.30 patch, Pawel Kot, 2011/10/20
- Re: gnokii-0.6.30 patch, Alexander V. Lukyanov, 2011/10/20
- Re: gnokii-0.6.30 patch, Pawel Kot, 2011/10/23