Re: [PATCH] nfsd: make strdup_if_nonnull static

[NeilBrown <neilb@suse.com>]

[-- 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 --]