[asterisk-dev] tilghman: trunk r193757 - /trunk/apps/app_voicemail.c

[Russell Bryant]

On May 11, 2009, at 6:04 PM, SVN commits to the Digium repositories  
wrote:

> Author: tilghman
> Date: Mon May 11 18:04:14 2009
> New Revision: 193757
>
> URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=193757
> Log:
> Found and fixed a memory leak
>
> Modified:
>    trunk/apps/app_voicemail.c
>
> Modified: trunk/apps/app_voicemail.c
> URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_voicemail.c?view=diff&rev=193757&r1=193756&r2=193757
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- trunk/apps/app_voicemail.c (original)
> +++ trunk/apps/app_voicemail.c Mon May 11 18:04:14 2009
> @@ -415,6 +415,8 @@
> #define ERROR_LOCK_PATH  -100
>
>
> +AST_THREADSTORAGE(voicemail_extension_list);
> +
> enum {
> 	NEW_FOLDER,
> 	OLD_FOLDER,
> @@ -5074,7 +5076,7 @@
> 	char fmt[80];
> 	char *context;
> 	char ecodes[17] = "#";
> -	struct ast_str *tmp = ast_str_create(16);
> +	struct ast_str *tmp =  
> ast_str_thread_get(&voicemail_extension_list, 16);
> 	char *tmpptr;
> 	struct ast_vm_user *vmu;
> 	struct ast_vm_user svm;


I'm concerned about the overuse of thread local buffers throughout the  
code base.

Based on the commit message, it seems that in this case it was simply  
a hack to avoid having to ensure ast_free() is called in the right  
places.  After looking at app_voicemail, I can't find a better  
explanation for it.  leave_voicemail() is not a function called many  
times within the same thread.

So, I think thread storage in this case is way overkill.  Thoughts?

--
Russell Bryant
Digium, Inc. | Senior Software Engineer, Open Source Team Lead
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: www.digium.com & www.asterisk.org