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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <[email protected]>
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 <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
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;
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 <[email protected]>
I probably should have stuck that in there in the first place, just in
case.
Thanks,
NeilBrown
>
> --b.
>
> commit 3c14417a48da
> Author: NeilBrown <[email protected]>
> 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 <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
>
> 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 [email protected]
> 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 <[email protected]>
>
> 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.