Re: [PATCH] ftrace: return error instead of 12 bytes read

[scameron@beardog.cce.hp.com]

On Wed, Nov 18, 2009 at 05:18:53AM -0500, Dan Merillat wrote:
> On Thu, Nov 12, 2009 at 8:45 AM, Ingo Molnar <mingo@elte.hu> wrote:
> 
> > Even in filesystems, ~80% of the cases use proper negative values:
> >
> >  $ git grep 'return -E' fs/ | wc -l
> >  4540
> >  $ git grep 'return E' fs/ | wc -l
> >  895
> 
> Except....
> fs/9p/fid.c:                            return ERR_PTR(-EPERM);
> 
> try this:
> 
> $ git grep "return E[A-Z]*;" | grep -v EOF | grep -v ERROR | wc -l
> 138
> $ git grep "return -E[A-Z]*;"| wc -l
> 57285
> 
> 2 of those are in Documentation/ and 2 are comments from a quick
> glance.   134 uses of positive error returns, _74_ of which are in
> fs/xfs, 24 in bluetooth, and the rest scattered randomly around the
> kernel.
> 
> 134 positive error returns  vs 57881 negative?  The style is so
> strongly ingrained in the kernel (And I'd bet an audit of those
> remaning 134 would find at least one bug) that it'd be a good janitor
> task to go through and switch them.
> 
> 
> This is one example - a function that returns either ENXIO or 0, and
> the lone caller explicitly tests and flips the sign.
> 
> (Not signing off on this!!!!  but CCing the relevant people for this driver)
> 
> diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
> index 6399e50..79867d6 100644
> --- a/drivers/block/cciss.c
> +++ b/drivers/block/cciss.c
> @@ -428,7 +428,7 @@ cciss_proc_write(struct file *file, const char __user *buf,
> 
>                 rc = cciss_engage_scsi(h->ctlr);
>                 if (rc != 0)
> -                       err = -rc;
> +                       err = rc;
>                 else
>                         err = length;
>         } else
> diff --git a/drivers/block/cciss_scsi.c b/drivers/block/cciss_scsi.c
> index 3315268..4af3085 100644
> --- a/drivers/block/cciss_scsi.c
> +++ b/drivers/block/cciss_scsi.c
> @@ -1547,7 +1547,7 @@ cciss_engage_scsi(int ctlr)
>         if (sa->registered) {
>                 printk("cciss%d: SCSI subsystem already engaged.\n", ctlr);
>                 spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
> -               return ENXIO;
> +               return -ENXIO;
>         }
>         sa->registered = 1;
>         spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);

I sent a patch to fix this already.

http://groups.google.com/group/linux.kernel/browse_thread/thread/40b43d35dff0f30a/0a073fc8014547e0?hl=en&lnk=gst&q=cciss+weird#0a073fc8014547e0

-- steve