[asterisk-dev] Re: [svn-commits] rizzo: trunk r48084 - /trunk/main/utils.c

[Russell Bryant]

svn-commits at lists.digium.com wrote:
> Author: rizzo
> Date: Tue Nov 28 08:07:09 2006
> New Revision: 48084
> 
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=48084
> Log:
> some simplifications to
> ast_dynamic_str_thread_build_va_couldnt_we_choose_a_shorter_name()

hahaha ... you made me laugh in the middle of class.  :)

> 
> I am unsure whether the truncation of the string in case of a failed
> attempt should be done unconditionally. See the XXX mark.
> 
> Russel, ideas ?
> 
> 
> Modified:
>     trunk/main/utils.c
> 
> Modified: trunk/main/utils.c
> URL: http://svn.digium.com/view/asterisk/trunk/main/utils.c?view=diff&rev=48084&r1=48083&r2=48084
> ==============================================================================
> --- trunk/main/utils.c (original)
> +++ trunk/main/utils.c Tue Nov 28 08:07:09 2006
> @@ -975,27 +975,29 @@
>  int ast_dynamic_str_thread_build_va(struct ast_dynamic_str **buf, size_t max_len,
>  	struct ast_threadstorage *ts, int append, const char *fmt, va_list ap)
>  {
> -	int res;
> +	int res, need;
>  	int offset = (append && (*buf)->len) ? strlen((*buf)->str) : 0;
>  
>  	res = vsnprintf((*buf)->str + offset, (*buf)->len - offset, fmt, ap);
>  
> +	need = res + offset + 1;
>  	/* Check to see if there was not enough space in the string buffer to prepare
>  	 * the string.  Also, if a maximum length is present, make sure the current
>  	 * length is less than the maximum before increasing the size. */
> -	if ((res + offset + 1) > (*buf)->len && (max_len ? ((*buf)->len < max_len) : 1)) {
> +	if (need > (*buf)->len && (max_len == 0 || (*buf)->len < max_len) ) {
>  		/* Set the new size of the string buffer to be the size needed
>  		 * to hold the resulting string (res) plus one byte for the
>  		 * terminating '\0'.  If this size is greater than the max, set
>  		 * the new length to be the maximum allowed. */
> -		if (max_len)
> -			(*buf)->len = ((res + offset + 1) < max_len) ? (res + offset + 1) : max_len;
> -		else
> -			(*buf)->len = res + offset + 1;
> -
> -		if (!(*buf = ast_realloc(*buf, (*buf)->len + sizeof(*(*buf)))))
> +		if (max_len && max_len < need)
> +			need = max_len;
> +
> +		*buf = ast_realloc(*buf, need + sizeof(struct ast_dynamic_str));
> +		if (*buf == NULL)
>  			return AST_DYNSTR_BUILD_FAILED;
> -
> +		(*buf)->len = need;
> +
> +		/* Truncate the previous attempt. XXX this should be unconditional */
>  		if (append)
>  			(*buf)->str[offset] = '\0';

Well, it's not actually needed unless doing an append operation.  This is 
because, when appending an existing string, it first does a strlen() to figure 
out the offset to begin the write.  If doing a write from the beginning, the 
write will be at the beginning again no matter what.

It certainly wouldn't hurt anything to make it unconditional, especially if for 
some odd reason, this function was used directly instead of through the 
ast_dynamic_str_thread_(set|append)_va() macros, which ensure that it gets 
called the second time when necessary.

Sorry for the long names.  :)

-- 
Russell Bryant
Software Engineer
Digium, Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: russell.vcf
Type: text/x-vcard
Size: 278 bytes
Desc: not available
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20061128/13410b73/russell.vcf