All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.