Found using coccinelle. It suggested kmalloc/strcpy should be replaced
with kstrdup, but the entire function can be replaced by kstrdup.
Signed-off-by: Alexandru Juncu <[email protected]>
---
drivers/staging/lustre/lustre/libcfs/libcfs_string.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
index 9edccc9..4dba304 100644
--- a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
+++ b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
@@ -135,18 +135,7 @@ EXPORT_SYMBOL(cfs_str2mask);
/* Duplicate a string in a platform-independent way */
char *cfs_strdup(const char *str, u_int32_t flags)
{
- size_t lenz; /* length of str + zero byte */
- char *dup_str;
-
- lenz = strlen(str) + 1;
-
- dup_str = kmalloc(lenz, flags);
- if (dup_str == NULL)
- return NULL;
-
- memcpy(dup_str, str, lenz);
-
- return dup_str;
+ return kstrdup(str, flags);
}
EXPORT_SYMBOL(cfs_strdup);
--
1.8.1.2
On Fri, Jul 19, 2013 at 5:45 PM, Alexandru Juncu <[email protected]> wrote:
> Found using coccinelle. It suggested kmalloc/strcpy should be replaced
> with kstrdup, but the entire function can be replaced by kstrdup.
>
> Signed-off-by: Alexandru Juncu <[email protected]>
> ---
> drivers/staging/lustre/lustre/libcfs/libcfs_string.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
> index 9edccc9..4dba304 100644
> --- a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
> +++ b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
> @@ -135,18 +135,7 @@ EXPORT_SYMBOL(cfs_str2mask);
> /* Duplicate a string in a platform-independent way */
> char *cfs_strdup(const char *str, u_int32_t flags)
> {
> - size_t lenz; /* length of str + zero byte */
> - char *dup_str;
> -
> - lenz = strlen(str) + 1;
> -
> - dup_str = kmalloc(lenz, flags);
> - if (dup_str == NULL)
> - return NULL;
> -
> - memcpy(dup_str, str, lenz);
> -
> - return dup_str;
> + return kstrdup(str, flags);
> }
> EXPORT_SYMBOL(cfs_strdup);
It would be better if you replaced the calls to cfs_strdup() with
kstrdup() and got rid of cfs_strdup() altogether.
On 19 July 2013 18:08, Pekka Enberg <[email protected]> wrote:
> On Fri, Jul 19, 2013 at 5:45 PM, Alexandru Juncu <[email protected]> wrote:
>> Found using coccinelle. It suggested kmalloc/strcpy should be replaced
>> with kstrdup, but the entire function can be replaced by kstrdup.
>>
>> Signed-off-by: Alexandru Juncu <[email protected]>
>> ---
>> drivers/staging/lustre/lustre/libcfs/libcfs_string.c | 13 +------------
>> 1 file changed, 1 insertion(+), 12 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
>> index 9edccc9..4dba304 100644
>> --- a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
>> +++ b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
>> @@ -135,18 +135,7 @@ EXPORT_SYMBOL(cfs_str2mask);
>> /* Duplicate a string in a platform-independent way */
>> char *cfs_strdup(const char *str, u_int32_t flags)
>> {
>> - size_t lenz; /* length of str + zero byte */
>> - char *dup_str;
>> -
>> - lenz = strlen(str) + 1;
>> -
>> - dup_str = kmalloc(lenz, flags);
>> - if (dup_str == NULL)
>> - return NULL;
>> -
>> - memcpy(dup_str, str, lenz);
>> -
>> - return dup_str;
>> + return kstrdup(str, flags);
>> }
>> EXPORT_SYMBOL(cfs_strdup);
>
> It would be better if you replaced the calls to cfs_strdup() with
> kstrdup() and got rid of cfs_strdup() altogether.
I was thinking the same thing, but I hesitated because I didn't know
how used it was and I didn't want to break something.
On Fri, Jul 19, 2013 at 6:13 PM, Alexandru Juncu <[email protected]> wrote:
> I was thinking the same thing, but I hesitated because I didn't know
> how used it was and I didn't want to break something.
"git grep cfs_strdup" suggests that nobody uses it so you could just
remove it completely...
On 19 July 2013 18:21, Pekka Enberg <[email protected]> wrote:
> On Fri, Jul 19, 2013 at 6:13 PM, Alexandru Juncu <[email protected]> wrote:
>> I was thinking the same thing, but I hesitated because I didn't know
>> how used it was and I didn't want to break something.
>
> "git grep cfs_strdup" suggests that nobody uses it so you could just
> remove it completely...
Did a grep on the tree and yes, it's not used. But It's in a staging
directory so maybe the rest of the code has not been pushed in yet.
On Fri, Jul 19, 2013 at 06:29:34PM +0300, Alexandru Juncu wrote:
> On 19 July 2013 18:21, Pekka Enberg <[email protected]> wrote:
> > On Fri, Jul 19, 2013 at 6:13 PM, Alexandru Juncu <[email protected]> wrote:
> >> I was thinking the same thing, but I hesitated because I didn't know
> >> how used it was and I didn't want to break something.
> >
> > "git grep cfs_strdup" suggests that nobody uses it so you could just
> > remove it completely...
>
> Did a grep on the tree and yes, it's not used. But It's in a staging
> directory so maybe the rest of the code has not been pushed in yet.
Doesn't matter, if there are no users, please just remove it.
thanks,
greg k-h
On Fri, 2013-07-19 at 08:46 -0700, Greg Kroah-Hartman wrote:
> Doesn't matter, if there are no users, please just remove it.
Is that, basically, your approach to staging cleanups?
I ask because I noticed that "drivers/staging/lustre/lustre/ptlrpc/gss/"
is only built if CONFIG_PTLRPC_GSS is set. But the corresponding Kconfig
symbol is nowhere to be found.
Or do you prefer that I poke the people involved in (this case in)
Lustre before suggesting a drastic cleanup like that?
Paul Bolle
On 19 July 2013 19:07, Paul Bolle <[email protected]> wrote:
> On Fri, 2013-07-19 at 08:46 -0700, Greg Kroah-Hartman wrote:
>> Doesn't matter, if there are no users, please just remove it.
>
> Is that, basically, your approach to staging cleanups?
>
> I ask because I noticed that "drivers/staging/lustre/lustre/ptlrpc/gss/"
> is only built if CONFIG_PTLRPC_GSS is set. But the corresponding Kconfig
> symbol is nowhere to be found.
>
> Or do you prefer that I poke the people involved in (this case in)
> Lustre before suggesting a drastic cleanup like that?
Thanks for the comments!
The comment for that function is "Duplicate a string in a
platform-independent way" so it might be because it's called by code
that also run on non-Linux systems.
On Fri, Jul 19, 2013 at 6:46 PM, Greg Kroah-Hartman
<[email protected]> wrote:
> On Fri, Jul 19, 2013 at 06:29:34PM +0300, Alexandru Juncu wrote:
>> On 19 July 2013 18:21, Pekka Enberg <[email protected]> wrote:
>> > On Fri, Jul 19, 2013 at 6:13 PM, Alexandru Juncu <[email protected]> wrote:
>> >> I was thinking the same thing, but I hesitated because I didn't know
>> >> how used it was and I didn't want to break something.
>> >
>> > "git grep cfs_strdup" suggests that nobody uses it so you could just
>> > remove it completely...
>>
>> Did a grep on the tree and yes, it's not used. But It's in a staging
>> directory so maybe the rest of the code has not been pushed in yet.
>
> Doesn't matter, if there are no users, please just remove it.
Hi Greg,
There is already a similar patch for this https://lkml.org/lkml/2013/7/17/742,
already reviewed by Andreas Dilger.
thanks,
Daniel.
> -----Original Message-----
> From: Paul Bolle [mailto:[email protected]]
> Sent: Saturday, July 20, 2013 12:07 AM
> To: Greg Kroah-Hartman
> Cc: Alexandru Juncu; Pekka Enberg; andreas.dilger; Peng, Tao; driverdev; LKML
> Subject: Re: [PATCH] lustre:libcfs: remove redundant code.
>
> On Fri, 2013-07-19 at 08:46 -0700, Greg Kroah-Hartman wrote:
> > Doesn't matter, if there are no users, please just remove it.
>
> Is that, basically, your approach to staging cleanups?
>
> I ask because I noticed that "drivers/staging/lustre/lustre/ptlrpc/gss/"
> is only built if CONFIG_PTLRPC_GSS is set. But the corresponding Kconfig
> symbol is nowhere to be found.
>
That's my fault. We certainly _want_ the ptlrpc gss code. I'll get it fixed.
Thanks,
Tao
> Or do you prefer that I poke the people involved in (this case in)
> Lustre before suggesting a drastic cleanup like that?
>
>
> Paul Bolle
>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Mon, 2013-07-22 at 02:07 +0000, Peng, Tao wrote:
> Paul Bolle [mailto:[email protected]]
> > I ask because I noticed that "drivers/staging/lustre/lustre/ptlrpc/gss/"
> > is only built if CONFIG_PTLRPC_GSS is set. But the corresponding Kconfig
> > symbol is nowhere to be found.
> >
> That's my fault. We certainly _want_ the ptlrpc gss code. I'll get it fixed.
I'll stop worrying about it. But if little has changed by, say, early
2014, I might try to prod you again.
Thanks!
Paul Bolle