[CRIU] [PATCH 1/7] util: new string helpers introduced

[Pavel Emelyanov]

On 12/16/2015 11:49 PM, Stanislav Kinsburskiy wrote:
> 
> 
> 16.12.2015 18:44, Pavel Emelyanov пишет:
>> On 12/16/2015 06:33 PM, Stanislav Kinsburskiy wrote:
>>> This patch brings add_to_string() and construct_string() helpers.
>>> They allow to create a string with variable amount of parameters in sprintf()
>>> manner, but supporting string allocation (and reallocation if necessary)
>>>
>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>>> ---
>>>   include/util.h |    4 ++++
>>>   util.c         |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 65 insertions(+)
>>>
>>> diff --git a/include/util.h b/include/util.h
>>> index df708c8..5030065 100644
>>> --- a/include/util.h
>>> +++ b/include/util.h
>>> @@ -275,4 +275,8 @@ void tcp_nodelay(int sk, bool on);
>>>   void tcp_cork(int sk, bool on);
>>>   
>>>   const char *ns_to_string(unsigned int ns);
>>> +
>>> +char *add_to_string(char *str, const char *fmt, ...);
>>> +char *construct_string(const char *fmt, ...);
>>> +
>>>   #endif /* __CR_UTIL_H__ */
>>> diff --git a/util.c b/util.c
>>> index d56f973..a27b0b6 100644
>>> --- a/util.c
>>> +++ b/util.c
>>> @@ -51,6 +51,67 @@
>>>   
>>>   #define VMA_OPT_LEN	128
>>>   
>>> +static char *add_to_string_vargs(char *str, const char *fmt, va_list args)
>>> +{
>>> +	size_t offset = 0, delta;
>>> +	int ret;
>>> +	char *new;
>>> +	va_list tmp;
>>> +
>>> +	if (str)
>>> +		offset = strlen(str);
>>> +	delta = strlen(fmt) * 2;
>>> +
>>> +	do {
>>> +		ret = -ENOMEM;
>>> +		new = xrealloc(str, offset + delta);
>>> +		if (new) {
>>> +			va_copy(tmp, args);
>>> +			ret = vsnprintf(new + offset, delta, fmt, tmp);
>>> +			if (ret >= delta) {
>>> +				delta = ret +1;
>>> +				str = new;
>>> +				ret = 0;
>>> +			}
>>> +		}
>>> +	} while (ret == 0);
>>> +
>>> +	if (ret == -ENOMEM) {
>>> +		/* realloc failed. We must release former string */
>>> +		pr_err("Failed to allocate string\n");
>>> +		xfree(str);
>>> +	} else if (ret < 0) {
>>> +		/* vsnprintf failed */
>>> +		pr_err("Failed to print string\n");
>>> +		xfree(new);
>>> +		new = NULL;
>> When this routine returns NULL it's undefined whether the original
>> string is freed or not, so called is likely to leak memory.
> 
> Sorry, but I don't understand, where it happens (or maybe don't 
> understand, what do you mean).

Ah, not that's me misread the xfree-s and ret code checks. All seem to
be OK in this respect.

> If ret == -ENOMEM, it means realloc returned NULL.
>  From man page of realloc:
> "If realloc() fails, the original block is left untouched; it is not 
> freed or moved"
> Thus, previous allocation have to be freed.
> 
> If ret < 0, but !ENOMEM, then it means, that realloc succeeded, but 
> vsnprintf failed for some reason. And thus newly reallocated string have 
> to be freed.
> Another piece from realloc man page:
> "The realloc() function changes the size of the memory block pointed to 
> by ptr to size bytes".
> Thus freeing of "new" includes freeing of "str".
> 
> IOW, add_to_string always _modifies_ the the passed str pointer.. If it 
> failed, then the original string if freed is lost.
> 
> A few examples to illustrate:
> 
> Example #1 (wrong one!!!):
> 
> char *buf = "text";
> char *str;
> int nr = 10;
> 
> str = add_to_string(buf, " about %d hobbits", nr);
> 
> This is wrong, because (from man page):
> "Unless ptr is NULL, it must have been returned by an earlier call to 
> malloc(), calloc() or realloc().", while buf is located on stack
> 
> Example #2 (risky usage!!!):
> 
> char *buf;
> char *str;
> int nr = 10;
> 
> buf = malloc(10);
> strcpy(buf, "text");
> 
> str = add_to_string(buf, " about %d hobbits", nr);
> 
> In this example, if add_to_string() returned NULL, then buf is also 
> freed. This have to be taken into account.
> 
> Example #3 (correct):
> 
> char *buf;
> char *str;
> int nr = 10;
> 
> buf = malloc(10);
> strcpy(buf, "text");
> 
> str = add_to_string(NULL, "%s about %d hobbits", buf, nr);
> 
> Example #4 (also correct):
> 
> char *buf;
> char *str = NULL;
> int nr = 10;
> 
> buf = malloc(10);
> strcpy(buf, "text");
> 
> str = add_to_string(str, "%s about %d hobbits", buf, nr);
> 
> Example #5 (also correct):
> 
> char *buf;
> char *str;
> int nr = 10;
> 
> buf = malloc(10);
> strcpy(buf, "text");
> 
> buf = malloc(10);
> strcpy(buf, "This is ");
> 
> str = add_to_string(str, "%s about %d hobbits", buf, nr);
> 
>>> +	}
>>> +	return new;
>>> +}
>>> +
>>> +char *add_to_string(char *str, const char *fmt, ...)
>>> +{
>>> +	va_list args;
>>> +
>>> +	va_start(args, fmt);
>>> +	str = add_to_string_vargs(str, fmt, args);
>>> +	va_end(args);
>>> +
>>> +	return str;
>>> +}
>>> +
>>> +char *construct_string(const char *fmt, ...)
>>> +{
>>> +	va_list args;
>>> +	char *str;
>>> +
>>> +	va_start(args, fmt);
>>> +	str = add_to_string_vargs(NULL, fmt, args);
>>> +	va_end(args);
>>> +
>>> +	return str;
>>> +}
>>> +
>>>   static void vma_opt_str(const struct vma_area *v, char *opt)
>>>   {
>>>   	int p = 0;
>>>
>>> _______________________________________________
>>> CRIU mailing list
>>> CRIU at openvz.org
>>> https://lists.openvz.org/mailman/listinfo/criu
>>> .
>>>
> 
> .
>