C++isms have slipped into the code base and a bug in waypt_dupe

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

C++isms have slipped into the code base and a bug in waypt_dupe

Carl Reisinger
In two place in the current gdb.c there are variable declarations that,
while valid C++ and acceptable to the more recent gnu compilers, are not
acceptable to older C compilers. Lines 393 and 544 in gdb.c are the
problem areas.

I don't know what the policy is but someone might want to take a look at
this.

This compiles with the latest gcc34 but the gdb on my system does not
work with the debugging symbols produced.  GCC 2.95.4 barfs on gdb.c

I'm trying to debug gpsbabel to find the cause of:
gpsbabel -i gpx -f 2005.gpx -x stack,push,copy -x stack,pop,append -f
AddIn.gpx -f 57622.gpx  -x duplicate,shortname,all -o gpx -F Local.gpx
gpsbabel in free(): error: chunk is already free
Abort trap (core dumped)
*** Error code 134

After hacking up gdb.c I get the following backtrace:

(gdb) bt
#0  0x281377ac in kill () from /usr/lib/libc.so.4
#1  0x281790a6 in abort () from /usr/lib/libc.so.4
#2  0x28177b8d in isatty () from /usr/lib/libc.so.4
#3  0x28177bc3 in isatty () from /usr/lib/libc.so.4
#4  0x28178abe in isatty () from /usr/lib/libc.so.4
#5  0x28178d25 in free () from /usr/lib/libc.so.4
#6  0x804b52f in XFREE (mem=0x80b14a0, file=0x808f370 "waypt.c", line=280)
    at util.c:110
#7  0x804b033 in waypt_free (wpt=0x80b8400) at waypt.c:280
#8  0x808cbbf in duplicate_process () at duplicate.c:221
#9  0x804a36c in main (argc=19, argv=0xbfbff650) at main.c:227
(gdb)

The bug is in waypt_dupe(), it does not create a new wpt->gc_data.placer
string. Instead just using the results of the memcpy. This results in a
duplicate free in waypt_free at line 280 when it attempts to free
gc_data.placer.


Carl

The system is a FreeBSD 4.10 with gcc 2.95.4. The abort was generated
while running with a /etc/malloc.conf of "AJ".



-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
Gpsbabel-code mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
Reply | Threaded
Open this post in threaded view
|

Re: C++isms have slipped into the code base and a bug in waypt_dupe

Robert Lipe-2
On 6/30/05, Carl Reisinger <[hidden email]> wrote:
> In two place in the current gdb.c there are variable declarations that,
> while valid C++ and acceptable to the more recent gnu compilers, are not
> acceptable to older C compilers. Lines 393 and 544 in gdb.c are the
> problem areas.
>
> I don't know what the policy is but someone might want to take a look at
> this.

My policy is that those happen, but when they do they're fixed as soon
as I can find 'em.   We try to be C89 with a few exceptions for common
accepted extensions such as // comment and snprintf.

In fact, I fixed the very problem I think you're reporting against
gdb.c a short while ago:

revision 1.2
date: 2005/06/29 18:42:10;  author: robertl;  state: Exp;  lines: +64 -16
ISO C conformance fixes.

Can you run a 'cvs up' (1.3 of this file is current) and verify the
problem still exists?

> The bug is in waypt_dupe(), it does not create a new wpt->gc_data.placer
> string. Instead just using the results of the memcpy. This results in a
> duplicate free in waypt_free at line 280 when it attempts to free
> gc_data.placer.

Good analyis and the correct fix.  I've commited this and run your
test case through electric fence.   (We really should add a PQ to the
test case...)

Thanx!
RJL


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. <a href="http://ads.osdn.com/?ad_idt77&alloc_id492&op=click">http://ads.osdn.com/?ad_idt77&alloc_id492&op=click
_______________________________________________
Gpsbabel-code mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
Reply | Threaded
Open this post in threaded view
|

Re: C++isms have slipped into the code base and a bug in waypt_dupe

Carl Reisinger
Robert Lipe wrote:

>Can you run a 'cvs up' (1.3 of this file is current) and verify the
>problem still exists?
>
>  
>
Finally able to connect up to cvs. Yes, this is fixed

>>The bug is in waypt_dupe(), it does not create a new wpt->gc_data.placer
>>string. Instead just using the results of the memcpy. This results in a
>>duplicate free in waypt_free at line 280 when it attempts to free
>>gc_data.placer.
>>    
>>
>
>Good analyis and the correct fix.  I've commited this and run your
>test case through electric fence.   (We really should add a PQ to the
>test case...)
>
>  
>
I noticed the fix is committed but for some reason my update did not get
it (even after deleting waypt.c). I manually entered the fix and that is
fine also.

I did just notice something strange in the lines above the addition.
Above the new code to dup placer there is code to dup the desc_short and
desc_long. Sure, the code works, but I don't believe it is safe to be
duping the strings pointed to by the 'tmp' waypoint pointer. It would be
best if those two dups made use of the 'wpt' pointer as does the rest of
the code in waypt_dupe.


Carl




-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
Gpsbabel-code mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
Reply | Threaded
Open this post in threaded view
|

Re: C++isms have slipped into the code base and a bug in waypt_dupe

Robert Lipe
> I noticed the fix is committed but for some reason my update did not get
> it (even after deleting waypt.c). I manually entered the fix and that is

Sticky tags can trick you.   'cvs up -A' will reset that.

> I did just notice something strange in the lines above the addition.
> Above the new code to dup placer there is code to dup the desc_short and
> desc_long. Sure, the code works, but I don't believe it is safe to be

Actually, it probably doesn't really work right. Sure, it won't blow up,
but it won't do what's intended.  Very few modules use 'waypt_dupe' in
common practice, so it's not terribly shocking this hasn't been noticed.
In fact, only geocachers feeding a pocket query through one of those
paths would have been harmed.

I've applied the obvious fix.   Thanx.

RJL


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
Gpsbabel-code mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code