[systemd-devel] [PATCH] STACK (http://css.csail.mit.edu/stack/) found issues for unstable optimized code again.

[Stefan Beller]

The first problem arises in src/gudev/gudevdevice.c
In lines 688 and 882 we call the function split_at_whitespace,
which is just a wrapper around g_strsplit_set, but removes also
the empty strings.

Now in this wrapper function we have a 'gchar **result;' on
which we'll operate. In line 638 we start on removing
the empty strings by checking them one by one.
Before the for loop we could mentally add an
	assert(result);
because if this assertion doesn't hold, the access
of result[n] would segfault.

Now compilers, which optimize heavily such as gcc 4.8,
will take notes and remember this implicit assertion in
case it can optimize something later on based on
this assumption.

And indeed it can do so, when returning from the function
split_at_whitespace in lines 688 and 882. There are checks
for this assertion being violated:
  if (result == NULL)
    goto out;
But the assertion must hold, or we'd have been segfaulting
earlier. That means either, we can remove the two lines
  if (result == NULL)
    goto out;
or we forgot to check the results of g_strsplit_set properly.

Now it's time to read the documentation on that
https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html#g-strsplit-set
And as far as I understand that, g_strsplit_set will never return a plain
NULL, but at least a pointer to a NULL. So the implementation of the
wrapper seems alright and we can remove the superfluous lines checking
for NULL.
---
 src/gudev/gudevdevice.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/gudev/gudevdevice.c b/src/gudev/gudevdevice.c
index 6c9e0f5..57a47d9 100644
--- a/src/gudev/gudevdevice.c
+++ b/src/gudev/gudevdevice.c
@@ -685,8 +685,6 @@ g_udev_device_get_property_as_strv (GUdevDevice  *device,
     goto out;
 
   result = split_at_whitespace (s);
-  if (result == NULL)
-    goto out;
 
   if (device->priv->prop_strvs == NULL)
     device->priv->prop_strvs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_strfreev);
@@ -879,8 +877,6 @@ g_udev_device_get_sysfs_attr_as_strv (GUdevDevice  *device,
     goto out;
 
   result = split_at_whitespace (s);
-  if (result == NULL)
-    goto out;
 
   if (device->priv->sysfs_attr_strvs == NULL)
     device->priv->sysfs_attr_strvs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_strfreev);
-- 
1.9.rc2