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