[TRIVIAL] Cleanup in fs/devpts/inode.c

[TRIVIAL] Cleanup in fs/devpts/inode.c

[René Scharfe]

Hi,

this patch un-complicates a small piece of code of the dev/pts
filesystem and decreases the size of the object code by 8 bytes
for my build. Yay! :)

René


--- linux-2.5.66/fs/devpts/inode.c.orig	2003-01-17 13:54:26.000000000 +0100
+++ linux-2.5.66/fs/devpts/inode.c	2003-03-22 14:09:40.000000000 +0100
@@ -170,9 +170,8 @@
 	int err = register_filesystem(&devpts_fs_type);
 	if (!err) {
 		devpts_mnt = kern_mount(&devpts_fs_type);
-		err = PTR_ERR(devpts_mnt);
-		if (!IS_ERR(devpts_mnt))
-			err = 0;
+		if (IS_ERR(devpts_mnt))
+			err = PTR_ERR(devpts_mnt);
 	}
 	return err;
 }

Re: [TRIVIAL] Cleanup in fs/devpts/inode.c

[john]

> this patch un-complicates a small piece of code of the dev/pts
> filesystem and decreases the size of the object code by 8 bytes
> for my build. Yay! :)

> -		err = PTR_ERR(devpts_mnt);
> -		if (!IS_ERR(devpts_mnt))
> -			err = 0;
> +		if (IS_ERR(devpts_mnt))
> +			err = PTR_ERR(devpts_mnt);

Why not use

err = (IS_ERR(devpts_mnt) ? err = 0 : PTR_ERR(devpts_mnt));

?

John.

Re: [TRIVIAL] Cleanup in fs/devpts/inode.c

[Robert P. J. Day]

On Sat, 29 Mar 2003 john@grabjohn.com wrote:

> > this patch un-complicates a small piece of code of the dev/pts
> > filesystem and decreases the size of the object code by 8 bytes
> > for my build. Yay! :)
> 
> > -		err = PTR_ERR(devpts_mnt);
> > -		if (!IS_ERR(devpts_mnt))
> > -			err = 0;
> > +		if (IS_ERR(devpts_mnt))
> > +			err = PTR_ERR(devpts_mnt);
> 
> Why not use
> 
> err = (IS_ERR(devpts_mnt) ? err = 0 : PTR_ERR(devpts_mnt));

without my handy-dandy C reference book here, don't you have
that backwards?  

and is there some value to using "err = 0"  internally rather
than just "0"?

rday

p.s.  risking major embarrassment if i didn't remember my
C programming ...

Re: [TRIVIAL] Cleanup in fs/devpts/inode.c

[Robert Love]

On Sat, 2003-03-29 at 13:29, john@grabjohn.com wrote:

> Why not use
> 
> err = (IS_ERR(devpts_mnt) ? err = 0 : PTR_ERR(devpts_mnt));
> 
> ?

The point of his cleanup is that you already know err is zero (look
again).

You also got it backward.

	Robert Love

Re: [TRIVIAL] Cleanup in fs/devpts/inode.c

[Felipe Alfaro Solana]

On Sat, 2003-03-29 at 19:29, john@grabjohn.com wrote:
> > this patch un-complicates a small piece of code of the dev/pts
> > filesystem and decreases the size of the object code by 8 bytes
> > for my build. Yay! :)
> 
> > -		err = PTR_ERR(devpts_mnt);
> > -		if (!IS_ERR(devpts_mnt))
> > -			err = 0;
> > +		if (IS_ERR(devpts_mnt))
> > +			err = PTR_ERR(devpts_mnt);
> 
> Why not use
> 
> err = (IS_ERR(devpts_mnt) ? err = 0 : PTR_ERR(devpts_mnt));

Ugg! That's really ugly! ;-)

________________________________________________________________________
        Felipe Alfaro Solana
   Linux Registered User #287198
http://counter.li.org

Re: [TRIVIAL] Cleanup in fs/devpts/inode.c

[Alex Bligh - linux-kernel]

--On 29 March 2003 18:29 +0000 john@grabjohn.com wrote:

>> this patch un-complicates a small piece of code of the dev/pts
>> filesystem and decreases the size of the object code by 8 bytes
>> for my build. Yay! :)
>
>> -		err = PTR_ERR(devpts_mnt);
>> -		if (!IS_ERR(devpts_mnt))
>> -			err = 0;
>> +		if (IS_ERR(devpts_mnt))
>> +			err = PTR_ERR(devpts_mnt);
>
> Why not use
>
> err = (IS_ERR(devpts_mnt) ? err = 0 : PTR_ERR(devpts_mnt));

Perhaps because it inverts the logic, and has a superfluous
assignment causing a warning? :-) I think you meant:

  err = IS_ERR(devpts_mnt) ? PTR_ERR(devpts_mnt) : 0;

--
Alex Bligh

Re: [TRIVIAL] Cleanup in fs/devpts/inode.c

[john]

> >> this patch un-complicates a small piece of code of the dev/pts
> >> filesystem and decreases the size of the object code by 8 bytes
> >> for my build. Yay! :)
> >
> >> -		err = PTR_ERR(devpts_mnt);
> >> -		if (!IS_ERR(devpts_mnt))
> >> -			err = 0;
> >> +		if (IS_ERR(devpts_mnt))
> >> +			err = PTR_ERR(devpts_mnt);
> >
> > Why not use
> >
> > err = (IS_ERR(devpts_mnt) ? err = 0 : PTR_ERR(devpts_mnt));
> 
> Perhaps because it inverts the logic, and has a superfluous
> assignment causing a warning? :-)

Good point :-).

> I think you meant:
> 
>   err = IS_ERR(devpts_mnt) ? PTR_ERR(devpts_mnt) : 0;

Yep.

John.