2003-03-29 18:03:57

by Ren

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

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;
}

2003-03-29 18:15:35

by John Bradford

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

> 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.

2003-03-29 18:30:33

by Robert P. J. Day

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

On Sat, 29 Mar 2003 [email protected] 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 ...

2003-03-29 19:01:10

by Robert Love

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

On Sat, 2003-03-29 at 13:29, [email protected] 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

2003-03-29 19:49:15

by Felipe Alfaro Solana

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

On Sat, 2003-03-29 at 19:29, [email protected] 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

2003-03-29 20:02:58

by Alex Bligh - linux-kernel

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

--On 29 March 2003 18:29 +0000 [email protected] 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

2003-03-29 21:25:27

by John Bradford

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

> >> 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.