| Message ID | 20170331200909.GC8487@fieldses.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 31 2017, J. Bruce Fields wrote: > On Thu, Mar 30, 2017 at 05:50:47PM +1100, NeilBrown wrote: >> On Wed, Mar 29 2017, J. Bruce Fields wrote: >> >> > Thanks, applying for 4.12.--b. >> >> Ugh... does strdup_if_nonnull() actually help readability at all? >> kstrdup() already handles NULL fine, which is what the name seems to >> suggest is happening. >> I would think kstrdup_report_error() is a name that better reflects the >> function. >> >> Or just discard it: > > Yes, what was I thinking? Just didn't read kstrdup(), I guess. > Applying as follows. Thanks. Feel free to add Signed-off-by: NeilBrown <neilb@suse.com> I probably should have stuck that in there in the first place, just in case. Thanks, NeilBrown > > --b. > > commit 3c14417a48da > Author: NeilBrown <neilb@suse.com> > Date: Thu Mar 23 16:57:36 2017 +0800 > > nfsd4: remove pointless strdup_if_nonnull > > kstrdup() already checks for NULL. > > (Brought to our attention by Jason Yann noticing (from sparse output) > that it should have been declared static.) > > Reported-by: Jason Yan <yanaijie@huawei.com> > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index e9ef50addddb..78ff82123a1a 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -1912,28 +1912,17 @@ static void copy_clid(struct nfs4_client *target, struct nfs4_client *source) > target->cl_clientid.cl_id = source->cl_clientid.cl_id; > } > > -int strdup_if_nonnull(char **target, char *source) > -{ > - if (source) { > - *target = kstrdup(source, GFP_KERNEL); > - if (!*target) > - return -ENOMEM; > - } else > - *target = NULL; > - return 0; > -} > - > static int copy_cred(struct svc_cred *target, struct svc_cred *source) > { > int ret; > > - ret = strdup_if_nonnull(&target->cr_principal, source->cr_principal); > - if (ret) > - return ret; > - ret = strdup_if_nonnull(&target->cr_raw_principal, > - source->cr_raw_principal); > - if (ret) > - return ret; > + target->cr_principal = kstrdup(source->cr_principal, GFP_KERNEL); > + target->cr_raw_principal = kstrdup(source->cr_raw_principal, > + GFP_KERNEL); > + if ((source->cr_principal && ! target->cr_principal) || > + (source->cr_raw_principal && ! target->cr_raw_principal)) > + return -ENOMEM; > + > target->cr_flavor = source->cr_flavor; > target->cr_uid = source->cr_uid; > target->cr_gid = source->cr_gid; > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 03, 2017 at 12:15:55PM +1000, NeilBrown wrote: > On Fri, Mar 31 2017, J. Bruce Fields wrote: > > > On Thu, Mar 30, 2017 at 05:50:47PM +1100, NeilBrown wrote: > >> On Wed, Mar 29 2017, J. Bruce Fields wrote: > >> > >> > Thanks, applying for 4.12.--b. > >> > >> Ugh... does strdup_if_nonnull() actually help readability at all? > >> kstrdup() already handles NULL fine, which is what the name seems to > >> suggest is happening. > >> I would think kstrdup_report_error() is a name that better reflects the > >> function. > >> > >> Or just discard it: > > > > Yes, what was I thinking? Just didn't read kstrdup(), I guess. > > Applying as follows. > > > Thanks. > Feel free to add > Signed-off-by: NeilBrown <neilb@suse.com> > > I probably should have stuck that in there in the first place, just in > case. For a small patch from a known contributor I figure the risk of just assuming DCO case (c) is negligible. Anyway, added, thanks. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index e9ef50addddb..78ff82123a1a 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1912,28 +1912,17 @@ static void copy_clid(struct nfs4_client *target, struct nfs4_client *source) target->cl_clientid.cl_id = source->cl_clientid.cl_id; } -int strdup_if_nonnull(char **target, char *source) -{ - if (source) { - *target = kstrdup(source, GFP_KERNEL); - if (!*target) - return -ENOMEM; - } else - *target = NULL; - return 0; -} - static int copy_cred(struct svc_cred *target, struct svc_cred *source) { int ret; - ret = strdup_if_nonnull(&target->cr_principal, source->cr_principal); - if (ret) - return ret; - ret = strdup_if_nonnull(&target->cr_raw_principal, - source->cr_raw_principal); - if (ret) - return ret; + target->cr_principal = kstrdup(source->cr_principal, GFP_KERNEL); + target->cr_raw_principal = kstrdup(source->cr_raw_principal, + GFP_KERNEL); + if ((source->cr_principal && ! target->cr_principal) || + (source->cr_raw_principal && ! target->cr_raw_principal)) + return -ENOMEM; + target->cr_flavor = source->cr_flavor; target->cr_uid = source->cr_uid; target->cr_gid = source->cr_gid;