[libvirt] [PATCH] esx: Replace scanf with STRSKIP and strtok_r
Matthias Bolte
matthias.bolte at googlemail.com
Thu Apr 15 18:14:19 UTC 2010
2010/4/15 Eric Blake <eblake at redhat.com>:
> On 04/14/2010 06:34 PM, Matthias Bolte wrote:
>> This also fixes a portability problem with the %a format modifier.
>> %a is not portable and made esxDomainDumpXML fail at runtime in
>> MinGW builds.
>>
>> Pull in strtok_r from gnulib, because MinGW lacks it.
>> ---
>> bootstrap.conf | 1 +
>
> See my comments about moving this hunk into your patch for .gnulib.
>
>> @@ -60,8 +60,8 @@ esxSupportsLongMode(esxPrivate *priv)
>> esxVI_DynamicProperty *dynamicProperty = NULL;
>> esxVI_HostCpuIdInfo *hostCpuIdInfoList = NULL;
>> esxVI_HostCpuIdInfo *hostCpuIdInfo = NULL;
>> + esxVI_ParsedHostCpuIdInfo parsedHostCpuIdInfo;
>> char edxLongModeBit = '?';
>> - char edxFirstBit = '?';
>>
>> if (priv->supportsLongMode != esxVI_Boolean_Undefined) {
>> return priv->supportsLongMode;
>> @@ -96,23 +96,12 @@ esxSupportsLongMode(esxPrivate *priv)
>> for (hostCpuIdInfo = hostCpuIdInfoList; hostCpuIdInfo != NULL;
>> hostCpuIdInfo = hostCpuIdInfo->_next) {
>> if (hostCpuIdInfo->level->value == -2147483647) { /* 0x80000001 */
>> -#define _SKIP4 "%*c%*c%*c%*c"
>> -#define _SKIP12 _SKIP4":"_SKIP4":"_SKIP4
>> -
>> - /* Expected format: "--X-:----:----:----:----:----:----:----" */
>> - if (sscanf(hostCpuIdInfo->edx,
>> - "%*c%*c%c%*c:"_SKIP12":"_SKIP12":%*c%*c%*c%c",
>> - &edxLongModeBit, &edxFirstBit) != 2) {
>
> Oh my. I see why you want this replaced.
I thinks the ParsedHostCpuIdInfo will also come in handy for adding
detailed CPU feature/selection, because it allows simpler access to
the individual bits of the CPUID output.
>> + }
>> +
>> + return 0;
>> +
>> + failure:
>> + memset(parsedhostCpuIdInfo, 0, sizeof (esxVI_ParsedHostCpuIdInfo));
>
> In general, I prefer to see:
>
> memset(parsedhostCpuIdInfo, 0, sizeof *parsedhostCpuIdInfo);
>
> on the grounds that sizeof expr is more robust if expr changes types, in
> contrast to sizeof(type).
On the other hand sizeof expr is more prone to missing *.
Anyway, I switch it to sizeof expr.
> Many existing uses in libvirt also have the style
> sizeof(*parsedhostCpuIdInfo), although that makes it harder to see at a
> glance whether you are using sizeof expr or sizeof(type) for expressions
> that do not include *. So it's up to you whether to include () around
> the expr.
>
> ACK with that nit addressed.
>
Thanks, pushed.
Matthias
More information about the libvir-list
mailing list