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

[Pavel Emelyanov]

On 12/16/2015 11:21 PM, Stanislav Kinsburskiy wrote:
> 
> 
> 16.12.2015 18:41, 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;
>> Please, don't fight for every single byte. Calling construct_string("%d", 1234567890)
>> would result in 6 realloc-s and va_copy-s.
> 
> I think, you are mistaken. This "+1" is not a fight for 1 byte.
> 
>  From man page of vsnprintf:
> "The functions ... vsnprintf() ...  If the output was truncated ... 
> _return  number  of  characters_ (excluding  the  terminating  null 
> byte) which _would have been written_ to the final string if enough 
> space had been available. "

OK, I didn't know it. Please add code comment reminding people about this.

> IOW, vsnprintf returns the amount of space to allocate. And "+1" is the 
> space for '\0', which was excluded from the result.
> Thus, only two call for va_copy and realloc will happen for any string.
> This can be optimed a bit by calling first realloc with, say, 32 bytes. 
> But I though, that it doesn't make much sense.
> 
>>> +				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;
>>> +	}
>>> +	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
>>> .
>>>
> 
> .
>