On Tue, Jun 24, 2008 at 07:04:23AM +0200, Louis Rilling wrote: > Yup, I realized it this night (prevented me from sleeping by the way). > The previous arguments remain valid though. I hope that you won't discard > them because of this buggy one :) I'm quite happy with the current code, and I figure to leave it (taking a module_get() on item->owner). Thanks for thinking it out with me. > struct config_item * item = to_item(filp->f_path.dentry->d_parent); > [...] > if (item) > config_item_put(item); > > It looks strange: > 1/ either item may be NULL, and there is a probable memory leak because of the > reference grabbed in configfs_open_file(); > 2/ or item may never be NULL and this check is just useless and (at least for > me) misleading. > > If I understand correctly, option 2 is correct because I think you are right about (2), but I'm not changing it. That code comes from sysfs and I try not to change it unless its wrong. sysfs has actually changed their handling over time. What I try to do is keep track of such changes and sometimes bring them over if warranted. I haven't had a chance to look at the new way sysfs does it. Joel -- f/8 and be there. Joel Becker Principal Software Developer Oracle E-mail: joel.becker@xxxxxxxxxx Phone: (650) 506-8127 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html