On Fri, 07.02.14 16:39, Stefan Beller (stefanbel...@googlemail.com) wrote: > 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.
Not following. Your patch looks like an optimization, not a bugfix. But your mail subject line suggests there was something broken. Can you elaborate? I don't follow... Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
