Messages in this thread | | | Date | Mon, 19 Oct 2009 10:04:03 +0200 | From | Ingo Molnar <> | Subject | Re: nfs mount fail |
| |
* Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> > --- a/fs/nfs/super.c > > +++ b/fs/nfs/super.c > > @@ -1253,6 +1253,7 @@ static int nfs_parse_mount_options(char *raw, > > default: > > dfprintk(MOUNT, "NFS: unrecognized " > > "transport protocol\n"); > > + kfree(string); > > return 0; > > } > > break; > > There is a possible clean up there too. We can move the other kfree() > calls out of the inner switch statement, and coalesce them all into a > single call.
Correct - separately from the leak fix. (which potentially wants to go to -stable as well)
Plus it's not just the kfree() calls that can be refactored but also the ~25 match_strdup() call sites. Most of these repetitive sequences:
case Opt_retrans: string = match_strdup(args); if (string == NULL) goto out_nomem; rc = strict_strtoul(string, 10, &option); kfree(string); if (rc != 0 || option == 0) goto out_invalid_value; mnt->retrans = option; break;
could be pushed into a helper function, along the lines of:
case Opt_retrans: if (parse_opt(args, &mnt->retrans) < 0 || mnt->retrans == 0) goto out_invalid_value; break;
where the non-repetitive value checks can be done after a generic parse_opt(). (or something like that)
That makes it more readable as well, as the switch statement will only list true per option properties, with minimal repetitive patterns.
Ingo
| |