[PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Gabriel Devenyi]

This patch applies to 2.5.68. It converts all the remaining error returns to 
the new return -E form, this is in the KernelJanitor TODO list.

http://muss.mcmaster.ca/~devenyga/patch-linux-2.5.68-return-errors.patch

Please CC me with any discussion since I do not subscribe to lkml
-- 
Building the Future,
Gabriel Devenyi
devenyga@mcmaster.ca

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Randy.Dunlap]

On Tue, 29 Apr 2003 22:15:20 +0000 Gabriel Devenyi <devenyga@mcmaster.ca> wrote:

| This patch applies to 2.5.68. It converts all the remaining error returns to 
| the new return -E form, this is in the KernelJanitor TODO list.
| 
| http://muss.mcmaster.ca/~devenyga/patch-linux-2.5.68-return-errors.patch
| 
| Please CC me with any discussion since I do not subscribe to lkml
| -- 

I'd have to say that it really depends on whether the caller can
handle negative return values.  Did you check/audit the callers too?

If it's a well-defined Unix/Linux error code (like s/ENOMEM/-ENOMEM/),
this should be made to work (at least in most cases).

And don't change ones that use ERR_PTR, like this:

-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;


Local variable returns of positive/negative are probably not correct...
without auditing the callers, it's hard to say.  E.g.:

-	return ErrFlag;
+	return -ErrFlag;

(same type of change in DAC960 driver)


I'm a bit suspicious of:

-	return EOF;
+	return -EOF;

and

-		return E05;
+		return -E05;

It's not just a global search & replace...


One more thing... did you build and boot that modified kernel?
If so, did it build with the same number or fewer warnings than the
unmodified version?

--
~Randy

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Gabriel Devenyi]

Thanks for the suggestions, I'm kinda new at this and just following the TODO 
which unfortuately says "sed s/return EWHATEVER/return -EWHATEVER/". I'll 
work on checking the things you suggested. As for your other questions, the 
kernel did build but I didn't attempt to boot it, I'll be sure to do so in 
the future. Thanks for the encouragement.

P.S. Anyone who works on KernelJanitor, kj.pl is suggesting some of the things 
I'm changing which aparently I shouldn't.

Building the Future,
Gabriel Devenyi
devenyga@mcmaster.ca

On April 29, 2003 10:32 pm, you wrote:
> On Tue, 29 Apr 2003 22:15:20 +0000 Gabriel Devenyi <devenyga@mcmaster.ca> 
wrote:
> | This patch applies to 2.5.68. It converts all the remaining error returns
> | to the new return -E form, this is in the KernelJanitor TODO list.
> |
> | http://muss.mcmaster.ca/~devenyga/patch-linux-2.5.68-return-errors.patch
> |
> | Please CC me with any discussion since I do not subscribe to lkml
> | --
>
> I'd have to say that it really depends on whether the caller can
> handle negative return values.  Did you check/audit the callers too?
>
> If it's a well-defined Unix/Linux error code (like s/ENOMEM/-ENOMEM/),
> this should be made to work (at least in most cases).
>
> And don't change ones that use ERR_PTR, like this:
>
> -		return ERR_PTR(-ENOMEM);
> +		return -ENOMEM;
>
>
> Local variable returns of positive/negative are probably not correct...
> without auditing the callers, it's hard to say.  E.g.:
>
> -	return ErrFlag;
> +	return -ErrFlag;
>
> (same type of change in DAC960 driver)
>
>
> I'm a bit suspicious of:
>
> -	return EOF;
> +	return -EOF;
>
> and
>
> -		return E05;
> +		return -E05;
>
> It's not just a global search & replace...
>
>
> One more thing... did you build and boot that modified kernel?
> If so, did it build with the same number or fewer warnings than the
> unmodified version?
>
> --
> ~Randy

-- 

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Dave Jones]

On Tue, Apr 29, 2003 at 11:08:30PM +0000, Gabriel Devenyi wrote:

 > P.S. Anyone who works on KernelJanitor, kj.pl is suggesting some of the things 
 > I'm changing which aparently I shouldn't.

I'd take most things it says with a pinch of salt.
I wasn't kidding when I said I knew no perl at all before I wrote this.
I stopped hacking on it when Dan Carpenters smatch appeared, as it
was a more 'real' solution with a future.

kj.pl was a fun weekend hack.

		Dave

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[viro]

On Tue, Apr 29, 2003 at 10:15:20PM +0000, Gabriel Devenyi wrote:
> This patch applies to 2.5.68. It converts all the remaining error returns to 
> the new return -E form, this is in the KernelJanitor TODO list.
> 
> http://muss.mcmaster.ca/~devenyga/patch-linux-2.5.68-return-errors.patch
> 
> Please CC me with any discussion since I do not subscribe to lkml

Have you tried to compile the patched kernel?

Patch is bogus - most of the changes are s/return ERR_PTR(foo)/return foo/
with neither explanation nor change of function prototype not change of
other exits from these functions.

WTF was it intended to do?

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Gabriel Devenyi]

It was intended to do exactly what the KernelJanitor TODO and kj.pl script 
pointed out, but aparently there's more to it than that. (BTW it just says 
"sed s/return EWHATEVER/return -EWHATEVER/") Discouraging people with foul 
language isn't the best way to get more developers, this is only my first 
try.

-- 
Building the Future,
Gabriel Devenyi
devenyga@mcmaster.ca

On April 29, 2003 10:42 pm, you wrote:
> On Tue, Apr 29, 2003 at 10:15:20PM +0000, Gabriel Devenyi wrote:
> > This patch applies to 2.5.68. It converts all the remaining error returns
> > to the new return -E form, this is in the KernelJanitor TODO list.
> >
> > http://muss.mcmaster.ca/~devenyga/patch-linux-2.5.68-return-errors.patch
> >
> > Please CC me with any discussion since I do not subscribe to lkml
>
> Have you tried to compile the patched kernel?
>
> Patch is bogus - most of the changes are s/return ERR_PTR(foo)/return foo/
> with neither explanation nor change of function prototype not change of
> other exits from these functions.
>
> WTF was it intended to do?

-- 
Building the Future,
Gabriel Devenyi
devenyga@mcmaster.ca

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Pavel Machek]

Hi!

> It was intended to do exactly what the KernelJanitor TODO and kj.pl script 
> pointed out, but aparently there's more to it than that. (BTW it just says 
> "sed s/return EWHATEVER/return -EWHATEVER/") Discouraging people with foul 
> language isn't the best way to get more developers, this is only my first 
> try.

That's Al Viro, it seems. He is hidding his real name? He's always
like that, and his flames are actually pretty nice reading. [And if
you did not even try to compile/boot that kernel, you deserve them.]


								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[viro]

On Tue, May 06, 2003 at 12:11:46AM +0200, Pavel Machek wrote:
> Hi!
> 
> > It was intended to do exactly what the KernelJanitor TODO and kj.pl script 
> > pointed out, but aparently there's more to it than that. (BTW it just says 
> > "sed s/return EWHATEVER/return -EWHATEVER/") Discouraging people with foul 
> > language isn't the best way to get more developers, this is only my first 
> > try.
> 
> That's Al Viro, it seems. He is hidding his real name? He's always
> like that

???

When the fsck had I ever stooped down to hiding my name?

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Martin Mares]

> When the fsck had I ever stooped down to hiding my name?

I guess Pavel means that From: in your latest submissions lacks
the full name.

				Have a nice fortnight
-- 
Martin `MJ' Mares   <mj@ucw.cz>   http://atrey.karlin.mff.cuni.cz/~mj/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
Current root password is "p3s5vwF50". Keep secret.

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Randy.Dunlap]

On Tue, 29 Apr 2003 23:08:30 +0000 Gabriel Devenyi <devenyga@mcmaster.ca> wrote:

| Thanks for the suggestions, I'm kinda new at this and just following the TODO 
| which unfortuately says "sed s/return EWHATEVER/return -EWHATEVER/". I'll 
| work on checking the things you suggested. As for your other questions, the 
| kernel did build but I didn't attempt to boot it, I'll be sure to do so in 
| the future. Thanks for the encouragement.
| 
| P.S. Anyone who works on KernelJanitor, kj.pl is suggesting some of the things 
| I'm changing which aparently I shouldn't.


The kernel-janitor TODO should be your guide.  However, it needs some
updating too, so the best thing to do is ask about things on
kernel-janitor-discuss@lists.sf.net before you spend time on them.

[item updates are welcome]

--
~Randy

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[viro]

On Tue, Apr 29, 2003 at 04:11:28PM -0700, Randy.Dunlap wrote:
> On Tue, 29 Apr 2003 23:08:30 +0000 Gabriel Devenyi <devenyga@mcmaster.ca> wrote:
> 
> | Thanks for the suggestions, I'm kinda new at this and just following the TODO 
> | which unfortuately says "sed s/return EWHATEVER/return -EWHATEVER/". I'll 
> | work on checking the things you suggested. As for your other questions, the 
> | kernel did build but I didn't attempt to boot it, I'll be sure to do so in 
> | the future. Thanks for the encouragement.
> | 
> | P.S. Anyone who works on KernelJanitor, kj.pl is suggesting some of the things 
> | I'm changing which aparently I shouldn't.
> 
> 
> The kernel-janitor TODO should be your guide.  However, it needs some
> updating too, so the best thing to do is ask about things on
> kernel-janitor-discuss@lists.sf.net before you spend time on them.
> 
> [item updates are welcome]

Well...  Turn that one into
	* try and convince XFS folks that they want to use negative numbers
for error values (will be tricky - they really don't like to diverge from
IRIX codebase)
	* ditto for JFS - again, a bunch of functions use positive error
values.
	* oprofile init on alpha should be returning negative in case of
failure to follow the common conventions.

AFAICS that's it.

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[John Levon]

On Wed, Apr 30, 2003 at 01:02:36AM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:

> 	* oprofile init on alpha should be returning negative in case of
> failure to follow the common conventions.

I noticed this yesterday and the change is in my tree already.

regards,
john

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Dave Kleikamp]

On Tuesday 29 April 2003 19:02, viro@parcelfarce.linux.theplanet.co.uk 
wrote:

> 	* ditto for JFS - again, a bunch of functions use positive error
> values.

Yeah, we've changed some of this from the way it was in OS/2, but we 
haven't gone through everything.  I'll put it on my todo list.

Thanks,
Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [PATCH] KernelJanitor: Convert remaining error returns to return -E Linux 2.5.68

[Randy.Dunlap]

> On Tue, Apr 29, 2003 at 04:11:28PM -0700, Randy.Dunlap wrote:
>> On Tue, 29 Apr 2003 23:08:30 +0000 Gabriel Devenyi <devenyga@mcmaster.ca>
>> wrote:
>>
>> | Thanks for the suggestions, I'm kinda new at this and just following the
>> TODO  | which unfortuately says "sed s/return EWHATEVER/return
>> -EWHATEVER/". I'll  | work on checking the things you suggested. As for
>> your other questions, the  | kernel did build but I didn't attempt to boot
>> it, I'll be sure to do so in  | the future. Thanks for the encouragement.
>> |
>> | P.S. Anyone who works on KernelJanitor, kj.pl is suggesting some of the
>> things  | I'm changing which aparently I shouldn't.
>>
>>
>> The kernel-janitor TODO should be your guide.  However, it needs some
>> updating too, so the best thing to do is ask about things on
>> kernel-janitor-discuss@lists.sf.net before you spend time on them.
>>
>> [item updates are welcome]
>
> Well...  Turn that one into
> 	* try and convince XFS folks that they want to use negative numbers
> for error values (will be tricky - they really don't like to diverge from
> IRIX codebase)
> 	* ditto for JFS - again, a bunch of functions use positive error
> values.
> 	* oprofile init on alpha should be returning negative in case of
> failure to follow the common conventions.
>
> AFAICS that's it.

OK, did that.  and thanks.

The updated KJ TODO will be available shortly.

~Randy