<div dir="ltr"><div class="gmail_extra">Aleksander, does v2 match what you expect?</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 28, 2017 at 6:59 PM, Ben Chan <span dir="ltr"><<a href="mailto:benchan@chromium.org" target="_blank">benchan@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">find_device_by_physdev_uid() expects a non-NULL UID of the physical<br>
device. However, mm_kernel_device_get_physdev_<wbr>uid() could potentially<br>
return NULL on MMKernelDeviceUdev if find_physical_gudevdevice() in<br>
mm-kernel-device-udev.c fails to find the physical device. When a NULL<br>
physical device UID is passed to find_device_by_physdev_uid() and used<br>
in g_hash_table_lookup(), it leads to a crash in g_str_hash(), which is<br>
a bit obscure.<br>
<br>
This patch updates kernel_device_get_physdev_uid(<wbr>) in<br>
mm-kernel-device-udev.c to fall back to use the device sysfs if the<br>
physical device sysfs isn't available, which is similar to how<br>
kernel_device_get_physdev_uid(<wbr>) mm-kernel-device-generic.c is<br>
implemented.<br>
---<br>
 src/kerneldevice/mm-kernel-<wbr>device-udev.c | 29 ++++++++++++++++++++---------<br>
 1 file changed, 20 insertions(+), 9 deletions(-)<br>
<br>
diff --git a/src/kerneldevice/mm-kernel-<wbr>device-udev.c b/src/kerneldevice/mm-kernel-<wbr>device-udev.c<br>
index 763ccf86..cd4f751f 100644<br>
--- a/src/kerneldevice/mm-kernel-<wbr>device-udev.c<br>
+++ b/src/kerneldevice/mm-kernel-<wbr>device-udev.c<br>
@@ -358,19 +358,30 @@ kernel_device_get_physdev_uid (MMKernelDevice *_self)<br>
     self = MM_KERNEL_DEVICE_UDEV (_self);<br>
<br>
     /* Prefer the one coming in the properties, if any */<br>
-    if (self->priv->properties)<br>
-        uid = mm_kernel_event_properties_<wbr>get_uid (MM_KERNEL_DEVICE_UDEV (self)->priv->properties);<br>
+    if (self->priv->properties) {<br>
+        if ((uid = mm_kernel_event_properties_<wbr>get_uid (self->priv->properties)) != NULL)<br>
+            return uid;<br>
+    }<br>
<br>
-    if (!uid) {<br>
-        ensure_physdev (self);<br>
-        if (!self->priv->physdev)<br>
-            return NULL;<br>
+    ensure_physdev (self);<br>
+    if (self->priv->physdev) {<br>
+        /* Try to load from properties set on the physical device */<br>
+        if ((uid = g_udev_device_get_property (self->priv->physdev, "ID_MM_PHYSDEV_UID")) != NULL)<br>
+            return uid;<br>
+<br>
+        /* Use physical device sysfs path, if any */<br>
+        if ((uid = g_udev_device_get_sysfs_path (self->priv->physdev)) != NULL)<br>
+            return uid;<br>
+    }<br>
<br>
-        uid = g_udev_device_get_property (self->priv->physdev, "ID_MM_PHYSDEV_UID");<br>
-        if (!uid)<br>
-            uid = g_udev_device_get_sysfs_path (self->priv->physdev);<br>
+    if (self->priv->device) {<br>
+        /* If there is no physical device sysfs path, use the device sysfs itself */<br>
+        if ((uid = g_udev_device_get_sysfs_path (self->priv->device)) != NULL)<br>
+            return uid;<br>
     }<br>
<br>
+    /* The caller of this function expects a non-NULL UID */<br>
+    g_assert (uid != NULL);<br>
     return uid;<br>
 }<br>
<span class="gmail-HOEnZb"><font color="#888888"><br>
--<br>
2.12.0.rc1.440.g5b76565f74-<wbr>goog<br>
<br>
______________________________<wbr>_________________<br>
ModemManager-devel mailing list<br>
<a href="mailto:ModemManager-devel@lists.freedesktop.org">ModemManager-devel@lists.<wbr>freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/modemmanager-<wbr>devel</a><br>
</font></span></blockquote></div><br></div></div>