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

[Stanislav Kinsburskiy]

16.12.2015 21:21, 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.
>

Sorry. I forgot my own code.
Initial allocation is done for strlen(str) * 2.
If it's not enought, then result of vsnprintf() is used to allocate a 
string of sufficient size.

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