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

[Stanislav Kinsburskiy]

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. "

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
>> .
>>