* [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy
@ 2017-07-07 14:45 Hari Prasath
2017-07-08 11:01 ` Sakari Ailus
0 siblings, 1 reply; 5+ messages in thread
From: Hari Prasath @ 2017-07-07 14:45 UTC (permalink / raw)
To: mchehab
Cc: gregkh, alan, rvarsha016, julia.lawall, singhalsimran0,
linux-media, devel, linux-kernel
kstrdup kernel primitive can be used to replace kmalloc followed by
string copy. This was reported by coccinelle tool
Signed-off-by: Hari Prasath <gehariprasath@gmail.com>
---
.../media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
index 34cc56f..68db87b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
@@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi, struct ia
)
{
char *namebuffer;
- int namelength = (int)strlen(name);
-
- namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
- if (namebuffer == NULL)
- return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
-
- memcpy(namebuffer, name, namelength + 1);
+ namebuffer = kstrdup(name, GFP_KERNEL);
+ if (!namebuffer)
+ return -ENOMEM;
bd->name = fw_minibuffer[index].name = namebuffer;
} else {
bd->name = name;
--
2.10.0.GIT
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy
2017-07-07 14:45 [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy Hari Prasath
@ 2017-07-08 11:01 ` Sakari Ailus
2017-07-09 12:26 ` hari prasath
0 siblings, 1 reply; 5+ messages in thread
From: Sakari Ailus @ 2017-07-08 11:01 UTC (permalink / raw)
To: Hari Prasath
Cc: mchehab, gregkh, alan, rvarsha016, julia.lawall, singhalsimran0,
linux-media, devel, linux-kernel
Hi Hari,
On Fri, Jul 07, 2017 at 08:15:21PM +0530, Hari Prasath wrote:
> kstrdup kernel primitive can be used to replace kmalloc followed by
> string copy. This was reported by coccinelle tool
>
> Signed-off-by: Hari Prasath <gehariprasath@gmail.com>
> ---
> .../media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> index 34cc56f..68db87b 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> @@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi, struct ia
> )
> {
> char *namebuffer;
> - int namelength = (int)strlen(name);
> -
> - namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
> - if (namebuffer == NULL)
> - return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
> -
> - memcpy(namebuffer, name, namelength + 1);
>
> + namebuffer = kstrdup(name, GFP_KERNEL);
> + if (!namebuffer)
> + return -ENOMEM;
The patch also changes the return value in error cases. I believe the
caller(s) expect to get errors in the IA_CCS_ERR_* range.
> bd->name = fw_minibuffer[index].name = namebuffer;
> } else {
> bd->name = name;
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy
2017-07-08 11:01 ` Sakari Ailus
@ 2017-07-09 12:26 ` hari prasath
2017-07-09 19:52 ` Sakari Ailus
0 siblings, 1 reply; 5+ messages in thread
From: hari prasath @ 2017-07-09 12:26 UTC (permalink / raw)
To: Sakari Ailus
Cc: mchehab, gregkh, Alan Cox, rvarsha016, Julia Lawall,
SIMRAN SINGHAL, linux-media, devel, linux-kernel
On 8 July 2017 at 16:31, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> Hi Hari,
>
> On Fri, Jul 07, 2017 at 08:15:21PM +0530, Hari Prasath wrote:
>> kstrdup kernel primitive can be used to replace kmalloc followed by
>> string copy. This was reported by coccinelle tool
>>
>> Signed-off-by: Hari Prasath <gehariprasath@gmail.com>
>> ---
>> .../media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 10 +++-------
>> 1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> index 34cc56f..68db87b 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> @@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi, struct ia
>> )
>> {
>> char *namebuffer;
>> - int namelength = (int)strlen(name);
>> -
>> - namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
>> - if (namebuffer == NULL)
>> - return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>> -
>> - memcpy(namebuffer, name, namelength + 1);
>>
>> + namebuffer = kstrdup(name, GFP_KERNEL);
>> + if (!namebuffer)
>> + return -ENOMEM;
>
> The patch also changes the return value in error cases. I believe the
> caller(s) expect to get errors in the IA_CCS_ERR_* range.
Hi,
In this particular case, the calling function just checks if it's not
success defined by a enum. I think returning -ENOMEM would not effect,
at least in this case.
- Hari Prasath
>
>> bd->name = fw_minibuffer[index].name = namebuffer;
>> } else {
>> bd->name = name;
>
> --
> Regards,
>
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
--
Regards,
G.E.Hari Prasath
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy
2017-07-09 12:26 ` hari prasath
@ 2017-07-09 19:52 ` Sakari Ailus
2017-07-10 6:22 ` hari prasath
0 siblings, 1 reply; 5+ messages in thread
From: Sakari Ailus @ 2017-07-09 19:52 UTC (permalink / raw)
To: hari prasath
Cc: mchehab, gregkh, Alan Cox, rvarsha016, Julia Lawall,
SIMRAN SINGHAL, linux-media, devel, linux-kernel
On Sun, Jul 09, 2017 at 05:56:15PM +0530, hari prasath wrote:
> On 8 July 2017 at 16:31, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > Hi Hari,
> >
> > On Fri, Jul 07, 2017 at 08:15:21PM +0530, Hari Prasath wrote:
> >> kstrdup kernel primitive can be used to replace kmalloc followed by
> >> string copy. This was reported by coccinelle tool
> >>
> >> Signed-off-by: Hari Prasath <gehariprasath@gmail.com>
> >> ---
> >> .../media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 10 +++-------
> >> 1 file changed, 3 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> >> index 34cc56f..68db87b 100644
> >> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> >> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> >> @@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi, struct ia
> >> )
> >> {
> >> char *namebuffer;
> >> - int namelength = (int)strlen(name);
> >> -
> >> - namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
> >> - if (namebuffer == NULL)
> >> - return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
> >> -
> >> - memcpy(namebuffer, name, namelength + 1);
> >>
> >> + namebuffer = kstrdup(name, GFP_KERNEL);
> >> + if (!namebuffer)
> >> + return -ENOMEM;
> >
> > The patch also changes the return value in error cases. I believe the
> > caller(s) expect to get errors in the IA_CCS_ERR_* range.
>
> Hi,
>
> In this particular case, the calling function just checks if it's not
> success defined by a enum. I think returning -ENOMEM would not effect,
> at least in this case.
It might not, but the function now returns both negative Posix and positive
CSS error codes. The CSS error codes could well be converted to Posix but
it should be done consistently and preferrably in a separate patch.
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy
2017-07-09 19:52 ` Sakari Ailus
@ 2017-07-10 6:22 ` hari prasath
0 siblings, 0 replies; 5+ messages in thread
From: hari prasath @ 2017-07-10 6:22 UTC (permalink / raw)
To: Sakari Ailus
Cc: mchehab, gregkh, Alan Cox, Varsha Rao, Julia Lawall,
SIMRAN SINGHAL, linux-media, devel, linux-kernel
On 10 July 2017 at 01:22, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> On Sun, Jul 09, 2017 at 05:56:15PM +0530, hari prasath wrote:
>> On 8 July 2017 at 16:31, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>> > Hi Hari,
>> >
>> > On Fri, Jul 07, 2017 at 08:15:21PM +0530, Hari Prasath wrote:
>> >> kstrdup kernel primitive can be used to replace kmalloc followed by
>> >> string copy. This was reported by coccinelle tool
>> >>
>> >> Signed-off-by: Hari Prasath <gehariprasath@gmail.com>
>> >> ---
>> >> .../media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 10 +++-------
>> >> 1 file changed, 3 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> >> index 34cc56f..68db87b 100644
>> >> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> >> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> >> @@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi, struct ia
>> >> )
>> >> {
>> >> char *namebuffer;
>> >> - int namelength = (int)strlen(name);
>> >> -
>> >> - namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
>> >> - if (namebuffer == NULL)
>> >> - return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>> >> -
>> >> - memcpy(namebuffer, name, namelength + 1);
>> >>
>> >> + namebuffer = kstrdup(name, GFP_KERNEL);
>> >> + if (!namebuffer)
>> >> + return -ENOMEM;
>> >
>> > The patch also changes the return value in error cases. I believe the
>> > caller(s) expect to get errors in the IA_CCS_ERR_* range.
>>
>> Hi,
>>
>> In this particular case, the calling function just checks if it's not
>> success defined by a enum. I think returning -ENOMEM would not effect,
>> at least in this case.
>
> It might not, but the function now returns both negative Posix and positive
> CSS error codes. The CSS error codes could well be converted to Posix but
> it should be done consistently and preferrably in a separate patch.
Hi Sakari, Thanks for your comments. I will stick with just replacing
with kstrdup and retain the original error return value. I will send a
v3.
Regards,
Hari
>
> --
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
--
Regards,
G.E.Hari Prasath
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-10 6:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-07 14:45 [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy Hari Prasath
2017-07-08 11:01 ` Sakari Ailus
2017-07-09 12:26 ` hari prasath
2017-07-09 19:52 ` Sakari Ailus
2017-07-10 6:22 ` hari prasath
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).