Re: [libvirt] [PATCH] conf: Add check to avoid a NULL compare forSysfsPath

On 01/18/2019 02:49 AM, Cheng Lin wrote: > If the two sysfs_path are both NULL, there may be an incorrect > object returned for virNodeDeviceObjListFindBySysfsPath(). > > This check exists in old interface virNodeDeviceFindBySysfsPath(). > e.g. > virNodeDeviceFindBySysfsPath(virNodeDeviceObjListPtr devs, > const char *sysfs_path) > { > ... > if ((devs->objs[i]->def->sysfs_path != NULL) && > (STREQ(devs->objs[i]->def->sysfs_path, sysfs_path))) { > ... > } > > Signed-off-by: Cheng Lin <cheng.lin130(a)zte.com.cn&gt; > --- > src/conf/virnodedeviceobj.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c > index c8ad131..2e40de5 100644 > --- a/src/conf/virnodedeviceobj.c > +++ b/src/conf/virnodedeviceobj.c > @@ -207,7 +207,8 @@ virNodeDeviceObjListFindBySysfsPathCallback(const void *payload, > int want = 0; > > virObjectLock(obj); - if (STREQ_NULLABLE(obj->def->sysfs_path, sysfs_path)) > + if ((obj->def->sysfs_path != NULL) && > + (STREQ_NULLABLE(obj->def->sysfs_path, sysfs_path))) > want = 1; > virObjectUnlock(obj); > return want; > Thanks, I reformatted to this and pushed: @@ -207,7 +207,8 @@ virNodeDeviceObjListFindBySysfsPathCallback(const void *payload, int want = 0; virObjectLock(obj); - if (STREQ_NULLABLE(obj->def->sysfs_path, sysfs_path)) + if (obj->def->sysfs_path && + STREQ_NULLABLE(obj->def->sysfs_path, sysfs_path)) want = 1; virObjectUnlock(obj); return want; What kind of devices are you seeing that don't have a sysfs_path I? Seems like there's lots of places internally we expect it to be there, but some others where we don't, so people must have hit similar issues before, but I don't think I see an example in the udev output on my machine

- Cole