[Xastir-Dev] Bus error happened again

[Curt Mills, WE7U]

On Thu, 20 Nov 2003, Jack Twilley wrote:

> I did a little more analysis this time.
>
> There were 219 stations according to the stations global.  There were
> 219 stations in the linked list.  The linked list ended in 0x0.  All
> is good there.
>
> Something about this part of the code concerns me.

-snip-

> The line "p_station = p_station->t_next;" gets run after
> "station_del_ptr(p_station);" when a station gets deleted.
>
> The last line of station_del_ptr(DataRow *p_name) is
> "delete_station_memory(p_name)".  The next to last line of
> delete_station_memory(DataRow *p_del) is "free(p_del)".
>
> I'm pretty sure this is bad, because it appears to leave p_station
> pointing to a chunk of memory which has been free()'d (and which now
> contains 0xd0).

We're talking about the db.c:check_station_remove() function.

Agreed.  Primo find!

That one can certainly cause a segfault on some systems as you
describe.

I was wondering if it could cause memory leaks as well while trying
to expire old stations from the database:  We step through our list
from oldest to newest, and the p_next pointer is supposed to point
to the next newest.  We iterate through the list deleting all that
are too old.  Looks like we delete _one_ record and that's it, but
"t_first" gets updated properly by remove_name()/remove_time(), so
we're ok there.  If "t_first" didn't get updated, we'd have a memory
leak.  Basically we're just inefficent with the current code, as it
only deletes one and quits.  Except in your case: Delete one and
crash!

I suspect the reason I never saw this problem on my systems could be
that the memory gets blanked to zero.  That would make me jump out
of the while loop early.  I'd only delete one old station per call
to check_station_remove().  Very inefficient, and might not keep up
depending on how often it gets called.

I did a tweak to db.c that should correct the problem.  We should
delete all expired stations each iteration, making the code more
efficient as well as fixing your pointer segfault.  Again, thanks
for finding that one!

-- 
Curt Mills, WE7U                    hacker_NO_SPAM_ at tc.fluke.com
Senior Methods Engineer/SysAdmin
"Lotto:    A tax on people who are bad at math!"
"Windows:  Microsoft's tax on computer illiterates!" -- WE7U
"The world DOES revolve around me:  I picked the coordinate system!"