Messages in this thread | | | Date | Wed, 11 Jun 2008 13:03:21 -0400 | From | Neil Horman <> | Subject | Re: [PATCH] shm: Remove silly double assignment |
| |
On Wed, Jun 11, 2008 at 03:45:06AM -0700, Andrew Morton wrote: > On Tue, 10 Jun 2008 08:53:39 -0400 Neil Horman <nhorman@tuxdriver.com> wrote: > > > Found a silly double assignment of err is do_shmat. Silly, but good to clean up > > the useless code. > > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > > > > diff --git a/ipc/shm.c b/ipc/shm.c > > index 554429a..d05f6b5 100644 > > --- a/ipc/shm.c > > +++ b/ipc/shm.c > > @@ -894,8 +894,6 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr) > > if (!sfd) > > goto out_put_dentry; > > > > - err = -ENOMEM; > > - > > file = alloc_file(path.mnt, path.dentry, f_mode, &shm_file_operations); > > if (!file) > > goto out_free; > > Not silly, really. Look: > > err = -ENOMEM; > if (expr1) > goto out; > > err = -ENOMEM; > if (expr2) > goto out; > > each of these two units is a separate, self-contained clause. Removing > the second assignment to `err' breaks that separation and will make one > clause undesirably dependent upon the other. yes, I see that keeping the second assignment lets you insert code in the middle without having up to update afterwards. I just don't think the redundant assignment is worth keeping just for that purpose.
> > Example: if someone later comes up and does > > err = -ENOMEM; > if (expr1) > goto out; > > + er = -EINVAL; > + if (expr3) > + goto out; > > if (expr2) > goto out; > > then whoops, it broke. > Yes, but that would be the fault of the subsequent patch, and if they couldn't see that problem, I fear that a double assignment isn't going to help them much.
> The compiler should optimise away the second assignment anyway. Agreed, its really not a big deal. If its not worth it, its fine with me. I was just being janitorial, and thought this looked rather redundant.
Best Neil
-- /**************************************************** * Neil Horman <nhorman@tuxdriver.com> * Software Engineer, Red Hat ****************************************************/
| |