* [PATCH] nfsd: make strdup_if_nonnull static @ 2017-03-23 8:57 Jason Yan 2017-03-29 21:51 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: Jason Yan @ 2017-03-23 8:57 UTC (permalink / raw) To: bfields, jlayton, linux-nfs; +Cc: miaoxie, zhaohongjiang, Jason Yan Fixes the following sparse warning: fs/nfsd/nfs4state.c:1915:5: warning: symbol 'strdup_if_nonnull' was not declared. Should it be static? Signed-off-by: Jason Yan <yanaijie@huawei.com> --- fs/nfsd/nfs4state.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index e9ef50a..59a9e30 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1912,7 +1912,7 @@ 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) +static int strdup_if_nonnull(char **target, char *source) { if (source) { *target = kstrdup(source, GFP_KERNEL); -- 2.5.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: make strdup_if_nonnull static 2017-03-23 8:57 [PATCH] nfsd: make strdup_if_nonnull static Jason Yan @ 2017-03-29 21:51 ` J. Bruce Fields 2017-03-30 6:50 ` NeilBrown 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2017-03-29 21:51 UTC (permalink / raw) To: Jason Yan; +Cc: jlayton, linux-nfs, miaoxie, zhaohongjiang Thanks, applying for 4.12.--b. On Thu, Mar 23, 2017 at 04:57:36PM +0800, Jason Yan wrote: > Fixes the following sparse warning: > > fs/nfsd/nfs4state.c:1915:5: warning: symbol 'strdup_if_nonnull' was not > declared. Should it be static? > > Signed-off-by: Jason Yan <yanaijie@huawei.com> > --- > fs/nfsd/nfs4state.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index e9ef50a..59a9e30 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -1912,7 +1912,7 @@ 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) > +static int strdup_if_nonnull(char **target, char *source) > { > if (source) { > *target = kstrdup(source, GFP_KERNEL); > -- > 2.5.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: make strdup_if_nonnull static 2017-03-29 21:51 ` J. Bruce Fields @ 2017-03-30 6:50 ` NeilBrown 2017-03-31 20:09 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: NeilBrown @ 2017-03-30 6:50 UTC (permalink / raw) To: J. Bruce Fields, Jason Yan; +Cc: jlayton, linux-nfs, miaoxie, zhaohongjiang [-- Attachment #1: Type: text/plain, Size: 2728 bytes --] 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: diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index e9ef50addddb..cf28d1056e70 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1912,28 +1912,14 @@ 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; + 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; - 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_flavor = source->cr_flavor; target->cr_uid = source->cr_uid; target->cr_gid = source->cr_gid; NeilBrown > > On Thu, Mar 23, 2017 at 04:57:36PM +0800, Jason Yan wrote: >> Fixes the following sparse warning: >> >> fs/nfsd/nfs4state.c:1915:5: warning: symbol 'strdup_if_nonnull' was not >> declared. Should it be static? >> >> Signed-off-by: Jason Yan <yanaijie@huawei.com> >> --- >> fs/nfsd/nfs4state.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index e9ef50a..59a9e30 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -1912,7 +1912,7 @@ 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) >> +static int strdup_if_nonnull(char **target, char *source) >> { >> if (source) { >> *target = kstrdup(source, GFP_KERNEL); >> -- >> 2.5.0 > -- > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: make strdup_if_nonnull static 2017-03-30 6:50 ` NeilBrown @ 2017-03-31 20:09 ` J. Bruce Fields 2017-04-03 2:15 ` NeilBrown 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2017-03-31 20:09 UTC (permalink / raw) To: NeilBrown; +Cc: Jason Yan, jlayton, linux-nfs, miaoxie, zhaohongjiang 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. --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; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: make strdup_if_nonnull static 2017-03-31 20:09 ` J. Bruce Fields @ 2017-04-03 2:15 ` NeilBrown 2017-04-03 2:39 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: NeilBrown @ 2017-04-03 2:15 UTC (permalink / raw) To: J. Bruce Fields Cc: Jason Yan, jlayton, linux-nfs, miaoxie, zhaohongjiang, Stephen Rothwell [-- Attachment #1: Type: text/plain, Size: 2782 bytes --] 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: make strdup_if_nonnull static 2017-04-03 2:15 ` NeilBrown @ 2017-04-03 2:39 ` J. Bruce Fields 0 siblings, 0 replies; 6+ messages in thread From: J. Bruce Fields @ 2017-04-03 2:39 UTC (permalink / raw) To: NeilBrown Cc: Jason Yan, jlayton, linux-nfs, miaoxie, zhaohongjiang, Stephen Rothwell 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-03 2:39 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-23 8:57 [PATCH] nfsd: make strdup_if_nonnull static Jason Yan 2017-03-29 21:51 ` J. Bruce Fields 2017-03-30 6:50 ` NeilBrown 2017-03-31 20:09 ` J. Bruce Fields 2017-04-03 2:15 ` NeilBrown 2017-04-03 2:39 ` J. Bruce Fields
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.