On Fri, Dec 09, 2022 at 05:43:54AM +0530, Viresh Kumar wrote:
On 08-12-22, 18:06, Anthony PERARD wrote:
Nit: Something like: const char check[] = "virtio,device"; const size_t checkl = sizeof(check) - 1; ... strncmp(tmp, check, checkl)... (or just strncmp(tmp, check, sizeof(check)-1)) would avoid issue with both string "virtio,device" potentially been different.
I think that is a generic problem with all the strings I am using. What about this diff over current patch ?
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h index cdd155d925c1..a062fca0e2bb 100644 --- a/tools/libs/light/libxl_internal.h +++ b/tools/libs/light/libxl_internal.h @@ -166,6 +166,11 @@ /* Convert pfn to physical address space. */ #define pfn_to_paddr(x) ((uint64_t)(x) << XC_PAGE_SHIFT) +/* Virtio device types */ +#define VIRTIO_DEVICE_TYPE_GENERIC "virtio,device" +#define VIRTIO_DEVICE_TYPE_GPIO "virtio,device22" +#define VIRTIO_DEVICE_TYPE_I2C "virtio,device29"
That a good idea.
/* logging */ _hidden void libxl__logv(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval, const char *file /* may be 0 */, int line /* ignored if !file */, diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c index 64cec989c674..183d9c906e27 100644 --- a/tools/libs/light/libxl_virtio.c +++ b/tools/libs/light/libxl_virtio.c @@ -62,7 +62,7 @@ static int libxl__virtio_from_xenstore(libxl__gc *gc, const char *libxl_path, libxl_device_virtio *virtio) { const char *be_path, *tmp = NULL;
- int rc;
- int rc, len = sizeof(VIRTIO_DEVICE_TYPE_GENERIC) - 1;
That `len` variable is initialized quite far away from where it's used, so ...
virtio->devid = devid; @@ -112,8 +110,8 @@ static int libxl__virtio_from_xenstore(libxl__gc *gc, const char *libxl_path, if (rc) goto out; if (tmp) {
... you could declare `len` here instead.
if (!strncmp(tmp, "virtio,device", strlen("virtio,device"))) {virtio->type = strdup(tmp);
if (!strncmp(tmp, VIRTIO_DEVICE_TYPE_GENERIC, len)) {virtio->type = libxl__strdup(NOGC, tmp); } else { return ERROR_INVAL; }
Otherwise, those change looks fine. Thanks,