Re: [RFC PATCH] convert uhci-hcd to use debugfs

[Greg KH]

On Fri, Dec 10, 2004 at 08:15:18AM -0600, Josh Boyer wrote:
> On Thu, 2004-12-09 at 18:55, Greg KH wrote:
> >  static int uhci_proc_open(struct inode *inode, struct file *file)
> >  {
> 
> Didn't want to rename this uhci_debug_open?

Yeah, I could :)

> > -#ifdef CONFIG_PROC_FS
> > -	ent = create_proc_entry(hcd->self.bus_name, S_IFREG|S_IRUGO|S_IWUSR, uhci_proc_root);
> > -	if (!ent) {
> > -		dev_err(uhci_dev(uhci), "couldn't create uhci proc entry\n");
> > +	dentry = debugfs_create_file(hcd->self.bus_name, S_IFREG|S_IRUGO|S_IWUSR, uhci_debugfs_root, uhci, &uhci_proc_operations);
> > +	if (!dentry) {
> 
> That won't work if debugfs is not configured.  You'll get back (void *)
> -ENODEV, which is not NULL.

That's exactly correct.  We do _not_ want NULL to be returned if debugfs
is not configured in.  We want to be able to detect if an error
happened, not if the feature was just not configured.  Otherwise, if we
did return NULL if debugfs was not enabled, this code would just fail
and the uhci startup would never happen.

This is why people have to wrap proc functions in #ifdef, and why no one
ever checks the return values from devfs calls.  It's a real problem and
I don't want to duplicate that again.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/