On Tue, Apr 25, 2017 at 01:10:26PM +0200, Martin Kletzander wrote:
It is no longer needed thanks to the great virfilemock.c. And this
s/mock/wrapper
- return 0; + +/* Arbitrarily sized number, feel free to change, but the function should be + * used for small, interface-like files, so it should not be huge (subjective) */ +#define VIR_FILE_READ_VALUE_STRING_MAX 4096
you didn't move the define :).
+ +/** + * virFileReadValueScaledInt: + * @value: pointer to unsigned long long int to be filled in with the value + * @format, ...: file to read from + * + * Read unsigned scaled int from @format and put it into @value. + * + * Return -2 for non-existing file, -1 on other errors and 0 if everything went + * fine. + */ +int +virFileReadValueScaledInt(unsigned long long *value, const char *format, ...) +{ + int ret = -1; + char *str = NULL; + char *endp = NULL; + char *path = NULL; + va_list ap; + + va_start(ap, format); + if (virVasprintf(&path, format, ap) < 0) { + va_end(ap); + goto cleanup; + } + va_end(ap); + + if (!virFileExists(path)) { + ret = -2; + goto cleanup; + } + + if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0) + goto cleanup; + + virStringTrimOptionalNewline(str); + + if (virStrToLong_ullp(str, &endp, 10, value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid unsigned scaled integer value '%s' in file '%s'"), + str, path); + goto cleanup; + } + + ret = virScaleInteger(value, endp, 1024, ULLONG_MAX); + cleanup: + VIR_FREE(path); + VIR_FREE(str); + return ret; }
[...]
int virHostCPUGetCore(unsigned int cpu, unsigned int *core) { - int ret = virSysfsGetCpuValueUint(cpu, "topology/core_id", core); + int ret = virFileReadValueUint(core, + "%s/cpu/cpu%u/topology/core_id", + SYSFS_SYSTEM_PATH, cpu);
/* If the file is not there, it's 0 */ if (ret == -2) @@ -268,7 +272,9 @@ virHostCPUGetSiblingsList(unsigned int cpu) virBitmapPtr ret = NULL; int rv = -1;
- rv = virSysfsGetCpuValueBitmap(cpu, "topology/thread_siblings_list", &ret); + rv = virFileReadValueBitmap(&ret, + "%s/cpu/cpu%u/topology/thread_siblings_list", + SYSFS_SYSTEM_PATH, cpu);
So, I like that you put the constant as argument to the formatting string instead of concatenation since v1...
if (rv == -2) { /* If the file doesn't exist, the threadis its only sibling */ ret = virBitmapNew(cpu + 1); @@ -615,7 +621,7 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo, /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the * core, node, socket, thread and topology information from /sys */ - if (virAsprintf(&sysfs_nodedir, "%s/node", virSysfsGetSystemPath()) < 0) + if (virAsprintf(&sysfs_nodedir, SYSFS_SYSTEM_PATH "/node") < 0)
In which case we should stay consistent and ^this should be adjusted accordingly - applies to the rest of the asprintfs below. ACK with the nits fixed. Erik