[lttng-dev] [RFC PATCH lttng-tools v3] lttng cli: Accept human readable sizes for --subbuf-size

[Simon Marchi]

On 9 April 2013 12:56, David Goulet <dgoulet at efficios.com> wrote:
>> @@ -77,3 +79,122 @@ void list_cmd_options(FILE *ofp, struct poptOption *options)
>>               }
>>       }
>>  }
>> +
>> +/**
>> + * Prints the error message corresponding to a regex error code.
>> + *
>> + * @param errcode    The error code.
>> + * @param regex              The regex object that produced the error code.
>> + */
>> +static void regex_print_error(int errcode, regex_t *regex)
>> +{
>> +     /* Get length of error message and allocate accordingly */
>> +     size_t length = regerror(errcode, regex, NULL, 0);
>> +     char *buffer = malloc(length);
>
> I know it's not _necessary_ but I would prefer to have the same std
> across the code. So, we usually declare the variables without
> assignation (that comes from a function call), then use assert() for
> given params (if needed) and finally used them.
>
> Here, declare length and buffer without assignation. Assert on "regex !=
> NULL" because a NULL value seems to me that it might break (or
> segfault). Finally, set length, check the return error code and set
> buffer checking also for errors.
>
> Use zmalloc also please.

Ok, no problem.

There is nothing specified for the return value of regerror so there
is nothing to check. Actually, it does abort() if you pass a wrong
error code...

> I wonder if these *_LOG2 should go in default.h ? Thoughts?

I see default.h as a place you put default arbitrary values for buffer
sizes, paths, names, etc. KIBI_LOG2 and al are not values you decide.
I think they are fine in utils.h.

> If regex was uninit like for instance on regcomp error, does this call
> behaves well ?

Good catch, changed.

"Ack" for all other comments.

Thanks,

Simon