[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 03/10] Refactor major.minor.micro version parsing into a function



Eric Blake wrote:
> On 03/30/2010 10:20 AM, Matthias Bolte wrote:
>> +/**
>> + * virParseVersionString:
>> + * @str: const char pointer to the version string
>> + * @version: unsigned long pointer to output the version number
>> + *
>> + * Parse an unsigned version number from a version string. Expecting
>> + * 'major.minor.micro' format, ignoring an optional suffix.
>> + *
>> + * The major, minor and micro numbers are encoded into a single version number:
>> + *
>> + *   1000000 * major + 1000 * minor + micro
>> + *
>> + * Returns the 0 for success, -1 for error.
>> + */
>> +int
>> +virParseVersionString(const char *str, unsigned long *version)
>> +{
>> +    unsigned int major, minor, micro;
>> +    char *tmp = (char *)str;
>> +
>> +    if (virStrToLong_ui(tmp, &tmp, 10, &major) < 0 || tmp == NULL ||
>
> Another instance (2 times) where the tmp==NULL check is spurious.  But I
> didn't see anything else wrong with this patch.
>
>> -            if (sscanf(version, "%ld.%ld.%ld", &major, &minor, &release) != 3) {
>> -                xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, "Couldn't get version info");
>> -                xen_string_string_map_free(result);
>> -                VIR_FREE(version);
>> -                return -1;
>> -            }
>> -            *hvVer = major * 1000000 + minor * 1000 + release;
>> -            VIR_FREE(version);
>> +            if (virParseVersionString(version, hvVer) < 0)
>> +                xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR,
>> +                                          "Couldn't parse version info");
>
> Not introduced by your patch, but should we be translating this error
> message?

Yes, definitely.
Good catch.

I added it to the list here:

diff --git a/cfg.mk b/cfg.mk
index 9b8ee00..45da56a 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -215,6 +215,7 @@ msg_gen_function += virXenInotifyError
 msg_gen_function += virXenStoreError
 msg_gen_function += virXendError
 msg_gen_function += vshCloseLogFile
+msg_gen_function += xenapiSessionErrorHandler
 msg_gen_function += xenUnifiedError
 msg_gen_function += xenXMError

Then ran "make syntax-check" and get 53 new violations.

BTW, I'm seeing a lot of these, too:

  nwfilter/nwfilter_ebiptables_driver.c:193: warning: format not a string literal and no format arguments [-Wformat-security]
  nwfilter/nwfilter_ebiptables_driver.c:204: warning: format not a string literal and no format arguments [-Wformat-security]
  nwfilter/nwfilter_ebiptables_dr...


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]