Re: [Openocd-development] Bug in core.c::jtag_unregister_event_callback()

[Paul Richards]

Hi Peter,

I just saw your mail after submitting a patch.  I kept the double 
pointer usage for consistency but simplified the logic.


On 2010/12/08 15:35, Peter Stuge wrote:


Should it? I don't think the following callback should be skipped
just because the current one is removed.


I believe you're correct, must have been getting late.  My patch doesn't 
advance the pointer and iterates once again with the updated value.


Regards,

Paul

___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Bug in core.c::jtag_unregister_event_callback()

[Peter Stuge]

Paul Richards wrote:
> 307 for (callbacks_p = &jtag_event_callbacks;
> 308 *callbacks_p != NULL;
> 309 callbacks_p = next)
..
> 311 next = &((*callbacks_p)->next);
> ...
> 316 if ((*callbacks_p)->callback == callback)
> 317 {
> 318 free(*callbacks_p);
> 319 *callbacks_p = *next;
> 320 }

**next seems excessive. *next would work..


> The assignment after the free() call is accessing the callback
> structure that has just been freed.

..and there would actually not be a problem then. Hopefully easy fix.

I guess someone wanted to use a lot of pointers. For modifying lists
I strongly prefer keeping a pointer direct to the previous entry,
rather than a **.


> Also, the assignment on line 319 doesn't account for the next
> increment in the for statement.

Should it? I don't think the following callback should be skipped
just because the current one is removed.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Bug in core.c::jtag_unregister_event_callback()

[Paul Richards]

Hi Øyvind

On 2010/12/08 3:58, Øyvind Harboe wrote:

Looks like bugs indeed...

How did you come across them?


The processor I am using (PXA16x) has TAPs (including the CPU TAP 
controller) that must be switched in via the default TAP.  When I 
enabled the CPU TAP post-init the enable event callback was being 
unregistered and this was generating a segmentation fault.


Thanks for the submission information, I'll submit shortly.  The changes 
are minor, but please let me know if there's anything I missed with the 
coding style, etc.


Regards,

Paul

___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Bug in core.c::jtag_unregister_event_callback()

[Øyvind Harboe]

Looks like bugs indeed...

How did you come across them?

> I have fixed and verified on my build but am unfamiliar with submission
> procedures.  I'm happy to contribute with a little guidance.


1. Get the latest version from git

2. checkout the master branch

git checkout origin/master

2. Modify the files that need modifying

3. Commit changes:

git add .
git commit --author="Paul Richards "

3b. if you need to add some more changes to the same commit,
you can amend the commit

git add .
git commit --amend

4. create patch and post file to list

git format-patch HEAD^

-- 
Øyvind Harboe

Can Zylin Consulting help on your project?

US toll free 1-866-980-3434 / International +47 51 63 25 00

http://www.zylin.com/zy1000.html
ARM7 ARM9 ARM11 XScale Cortex
JTAG debugger and flash programmer
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

[Openocd-development] Bug in core.c::jtag_unregister_event_callback()

[Paul Richards]

Hi,

I believe I've found a bug or two in jtag_unregister_event_callback().

273 
 
int jtag_register_event_callback(jtag_event_handler_t callback, void *priv)
274 
 
{

...
311 
 
next = &((*callbacks_p)->next);

...
316 
 
if ((*callbacks_p)->callback == callback)
317 
 
{
318 
 
free(*callbacks_p);
319 
 
*callbacks_p = *next;
320 
 
}


The assignment after the free() call is accessing the callback structure 
that has just been freed.  Also, the assignment on line 319 doesn't 
account for the next increment in the for statement.


307 
 
for (callbacks_p = &jtag_event_callbacks;
308 
 
*callbacks_p != NULL;
309 
 
callbacks_p = next)


I have fixed and verified on my build but am unfamiliar with submission 
procedures.  I'm happy to contribute with a little guidance.


Regards,

Paul

___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development