[Home]

Summary:ASTERISK-04158: cdr.c::ast_cdr_unregister() incorrect
Reporter:Luigi Rizzo (rizzo)Labels:
Date Opened:2005-05-13 08:36:57Date Closed:2008-01-15 15:34:28.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:(disclaimer on file)

i suppose the intended goal of the function is to unlink
an entry from the 'bes' list, and perhaps free the storage as well.
As it is now, the code does not update prev in the loop, so
it throws away the entire initial part of the list
(and as a side effect, also leaking memory).
These bugs would be less easy if we adopted list navigation macros such
as the ones suggested in ASTERISK-4224254, or possibly something better.

The way i would code the loop is
AST_SL_TRAVERSE_PREV(bes, i, prev, next) {
   if (!strcasecmp(name, i->name))
       break;
}
if (i) {
   if (prev)
       prev->next = i->next;
   else
       bes = i->next;
}
ast_mutex_unlock(&cdrlock);
if (i) {
   free(i);
   if (option_verbose > 1)
       bla bla bla
}
   free(i);
   
 
 
Comments:By: Kevin P. Fleming (kpfleming) 2005-05-14 22:38:05

I have rewritten the backend list management in cdr.c to use the linkedlists.h macros (which included adding some functionality to those macros).
By: Digium Subversion (svnbot) 2008-01-15 15:34:28.000-0600

Repository: asterisk
Revision: 5659

U   trunk/cdr.c
U   trunk/include/asterisk/linkedlists.h

------------------------------------------------------------------------
r5659 | kpfleming | 2008-01-15 15:34:27 -0600 (Tue, 15 Jan 2008) | 6 lines

various fixes:
 use linked list macros for managing backend list (inspired by bug ASTERISK-4158)
 use ast_copy_string instead of strncpy when appropriate
 minor fixes and formatting cleanup
 add AST_LIST_HEAD_STATIC and AST_LIST_REMOVE_CURRENT macros

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5659