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.
@@ -213,37 +217,34 @@ esxUtil_ParseDatastoreRelatedPath(const char
*datastoreRelatedPath,
return -1;
}
- /*
- * Parse string as '[<datastore>] <path>'. '%as' is
similar to '%s', but
- * sscanf() will allocate the memory for the string, so the caller doesn't
- * need to preallocate a buffer that's large enough.
- *
- * The s in '%as' can be replaced with a character set, e.g. [a-z].
- *
- * '%a[^]%]' matches <datastore>. '[^]%]' excludes ']'
from the accepted
- * characters, otherwise sscanf() wont match what it should.
- *
- * '%a[^\n]' matches <path>. '[^\n]' excludes '\n'
from the accepted
- * characters, otherwise sscanf() would only match up to the first space,
- * but spaces are valid in <path>.
- */
- if (sscanf(datastoreRelatedPath, "[%a[^]%]] %a[^\n]", datastoreName,
- &directoryAndFileName) != 2) {
+ if (esxVI_String_DeepCopyValue(©OfDatastoreRelatedPath,
+ datastoreRelatedPath) < 0) {
+ goto failure;
+ }
+
+ /* Expected format: '[<datastore>] <path>' */
+ if ((tmp = STRSKIP(copyOfDatastoreRelatedPath, "[")) == NULL ||
+ (preliminaryDatastoreName = strtok_r(tmp, "]", &saveptr)) == NULL
||
+ (directoryAndFileName = strtok_r(NULL, "", &saveptr)) == NULL) {
Even though it is more lines of code, it is much smaller based on fewer
justification comments, and possibly faster execution to boot (strtok_r
is certainly simpler to implement than sscanf). I like it.
+int
+esxVI_ParseHostCpuIdInfo(esxVI_ParsedHostCpuIdInfo *parsedhostCpuIdInfo,
+ esxVI_HostCpuIdInfo *hostCpuIdInfo)
+{
+ int expectedLength = 39; /* =
strlen("----:----:----:----:----:----:----:----"); */
+ char *input[4] = { hostCpuIdInfo->eax, hostCpuIdInfo->ebx,
+ hostCpuIdInfo->ecx, hostCpuIdInfo->edx };
+ char *output[4] = { parsedhostCpuIdInfo->eax, parsedhostCpuIdInfo->ebx,
+ parsedhostCpuIdInfo->ecx, parsedhostCpuIdInfo->edx };
+ const char *name[4] = { "eax", "ebx", "ecx",
"edx" };
+ int r, i, o;
+
+ memset(parsedhostCpuIdInfo, 0, sizeof (esxVI_ParsedHostCpuIdInfo));
+
+ parsedhostCpuIdInfo->level = hostCpuIdInfo->level->value;
+
+ for (r = 0; r < 4; ++r) {
+ if (strlen(input[r]) != expectedLength) {
+ ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _("HostCpuIdInfo register '%s' has an unexpected
length"),
+ name[r]);
+ goto failure;
+ }
+
+ /* Strip the ':' and invert the "bit" order from 31..0 to
0..31 */
+ for (i = 0, o = 31; i < expectedLength; i += 5, o -= 4) {
+ output[r][o] = input[r][i];
+ output[r][o - 1] = input[r][i + 1];
+ output[r][o - 2] = input[r][i + 2];
+ output[r][o - 3] = input[r][i + 3];
+
+ if (i + 4 < expectedLength && input[r][i + 4] != ':') {
+ ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _("HostCpuIdInfo register '%s' has an
unexpected format"),
+ name[r]);
+ goto failure;
+ }
+ }
I spent quite a few minutes looking at this, and didn't see any semantic
differences between this longer implementation and the original sscanf.
+ }
+
+ 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).
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.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org