Hi,
> > Hu? I see 8 call sites, and those should all be covered - maybe it's just
> > because I'm working on a somewhat old tree?
>
> The ninth site was the recursive call within create_path(). I see
> you've added unlock()s there now. I re-counted on a per-file basis
> and everything else looks covered; I probably miscounted before.
That unlock wasn't necessary before.
> I'm still concerned/interested about the effect on performance. But
I guess that somebody should try it out ;-)
> > - if (util_create_path(udev, p) != 0)
> > + if (util_create_path(udev, p) != 0) {
> > + util_path_unlock();
> > return -1;
> > + }
>
> Looks suspicious. It'll try to unlock even If the recursive
> util_create_path() bailed because it failed at util_path_lock().
s/even/only/, and that's the whole point of it.
This is not for unlocking the recursive lock - if util_create_path()
returns non-zero, it is guaranteed that it hasn't acquired a lock,
which is also why we have to drop our own lock before returning -1.
> > int util_delete_path(struct udev *udev, const char *path)
[...]
> Hmm, does this function ever return non-zero? Is it time to admit
> that it returns void?
Just my thoughts ;-)
And even if it returned anything else, nobody cares anyhow ...
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
