* [PATCH 7/7] staging: cxt1e1: remove unneeded a value
@ 2014-03-05 1:24 Daeseok Youn
2014-03-05 10:13 ` Tobias Klauser
0 siblings, 1 reply; 6+ messages in thread
From: Daeseok Youn @ 2014-03-05 1:24 UTC (permalink / raw)
To: gregkh
Cc: davem, sachin.kamat, shaun, dulshani.gunawardhana89, devel,
linux-kernel, ying.xue
It doesn't need to assign name array address to np pointer.
Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
drivers/staging/cxt1e1/linux.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
index 5bb42ae..cae8c66 100644
--- a/drivers/staging/cxt1e1/linux.c
+++ b/drivers/staging/cxt1e1/linux.c
@@ -205,15 +205,14 @@ status_t
c4_wq_port_init(mpi_t *pi)
{
- char name[16], *np; /* NOTE: name of the queue limited by system
+ char name[16]; /* NOTE: name of the queue limited by system
* to 10 characters */
if (pi->wq_port)
return 0; /* already initialized */
- np = name;
memset(name, 0, 16);
- sprintf(np, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
+ sprintf(name, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
#ifdef RLD_RESTART_DEBUG
pr_info(">> %s: creating workqueue <%s> for Port %d.\n",
--
1.7.4.4
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 7/7] staging: cxt1e1: remove unneeded a value
2014-03-05 1:24 [PATCH 7/7] staging: cxt1e1: remove unneeded a value Daeseok Youn
@ 2014-03-05 10:13 ` Tobias Klauser
2014-03-06 7:19 ` DaeSeok Youn
0 siblings, 1 reply; 6+ messages in thread
From: Tobias Klauser @ 2014-03-05 10:13 UTC (permalink / raw)
To: Daeseok Youn
Cc: gregkh, devel, shaun, sachin.kamat, linux-kernel,
dulshani.gunawardhana89, ying.xue, davem
On 2014-03-05 at 02:24:22 +0100, Daeseok Youn <daeseok.youn@gmail.com> wrote:
>
> It doesn't need to assign name array address to np pointer.
>
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> drivers/staging/cxt1e1/linux.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
> index 5bb42ae..cae8c66 100644
> --- a/drivers/staging/cxt1e1/linux.c
> +++ b/drivers/staging/cxt1e1/linux.c
> @@ -205,15 +205,14 @@ status_t
> c4_wq_port_init(mpi_t *pi)
> {
>
> - char name[16], *np; /* NOTE: name of the queue limited by system
> + char name[16]; /* NOTE: name of the queue limited by system
> * to 10 characters */
>
> if (pi->wq_port)
> return 0; /* already initialized */
>
> - np = name;
> memset(name, 0, 16);
This isn't necessary since s{,n}printf() adds a terminating '\0'.
> - sprintf(np, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
> + sprintf(name, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
Better use snprintf() here, even if the comment above claims the name
never to be no longer than 10 characters.
Cheers
Tobias
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 7/7] staging: cxt1e1: remove unneeded a value
2014-03-05 10:13 ` Tobias Klauser
@ 2014-03-06 7:19 ` DaeSeok Youn
2014-03-06 7:33 ` Tobias Klauser
0 siblings, 1 reply; 6+ messages in thread
From: DaeSeok Youn @ 2014-03-06 7:19 UTC (permalink / raw)
To: Tobias Klauser
Cc: Greg KH, devel, Shaun Laing, sachin.kamat, linux-kernel,
Dulshani Gunawardhana, ying.xue, David Miller
2014-03-05 19:13 GMT+09:00 Tobias Klauser <tklauser@distanz.ch>:
> On 2014-03-05 at 02:24:22 +0100, Daeseok Youn <daeseok.youn@gmail.com> wrote:
>>
>> It doesn't need to assign name array address to np pointer.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>> ---
>> drivers/staging/cxt1e1/linux.c | 5 ++---
>> 1 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
>> index 5bb42ae..cae8c66 100644
>> --- a/drivers/staging/cxt1e1/linux.c
>> +++ b/drivers/staging/cxt1e1/linux.c
>> @@ -205,15 +205,14 @@ status_t
>> c4_wq_port_init(mpi_t *pi)
>> {
>>
>> - char name[16], *np; /* NOTE: name of the queue limited by system
>> + char name[16]; /* NOTE: name of the queue limited by system
>> * to 10 characters */
>>
>> if (pi->wq_port)
>> return 0; /* already initialized */
>>
>> - np = name;
>> memset(name, 0, 16);
>
> This isn't necessary since s{,n}printf() adds a terminating '\0'.
Yes, I have looked at lib/vsprintf.c. I found it adds null to a string
in the end of vsnprintf() function.
I will remove memset() line.
>
>> - sprintf(np, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
>> + sprintf(name, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
>
> Better use snprintf() here, even if the comment above claims the name
> never to be no longer than 10 characters.
OK. I will replace sprintf with snprintf() and set a string length to "10".
Thanks for review.
Daeseok Youn.
>
> Cheers
> Tobias
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 7/7] staging: cxt1e1: remove unneeded a value
2014-03-06 7:19 ` DaeSeok Youn
@ 2014-03-06 7:33 ` Tobias Klauser
2014-03-06 7:47 ` DaeSeok Youn
0 siblings, 1 reply; 6+ messages in thread
From: Tobias Klauser @ 2014-03-06 7:33 UTC (permalink / raw)
To: DaeSeok Youn
Cc: Greg KH, devel, Shaun Laing, sachin.kamat, linux-kernel,
Dulshani Gunawardhana, ying.xue, David Miller
On 2014-03-06 at 08:19:19 +0100, DaeSeok Youn <daeseok.youn@gmail.com> wrote:
> 2014-03-05 19:13 GMT+09:00 Tobias Klauser <tklauser@distanz.ch>:
> > On 2014-03-05 at 02:24:22 +0100, Daeseok Youn <daeseok.youn@gmail.com> wrote:
> >>
> >> It doesn't need to assign name array address to np pointer.
> >>
> >> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> >> ---
> >> drivers/staging/cxt1e1/linux.c | 5 ++---
> >> 1 files changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
> >> index 5bb42ae..cae8c66 100644
> >> --- a/drivers/staging/cxt1e1/linux.c
> >> +++ b/drivers/staging/cxt1e1/linux.c
> >> @@ -205,15 +205,14 @@ status_t
> >> c4_wq_port_init(mpi_t *pi)
> >> {
> >>
> >> - char name[16], *np; /* NOTE: name of the queue limited by system
> >> + char name[16]; /* NOTE: name of the queue limited by system
> >> * to 10 characters */
> >>
> >> if (pi->wq_port)
> >> return 0; /* already initialized */
> >>
> >> - np = name;
> >> memset(name, 0, 16);
> >
> > This isn't necessary since s{,n}printf() adds a terminating '\0'.
> Yes, I have looked at lib/vsprintf.c. I found it adds null to a string
> in the end of vsnprintf() function.
> I will remove memset() line.
>
> >
> >> - sprintf(np, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
> >> + sprintf(name, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
> >
> > Better use snprintf() here, even if the comment above claims the name
> > never to be no longer than 10 characters.
> OK. I will replace sprintf with snprintf() and set a string length to "10".
It's probably fine to leave the string at length 16 (since there's also
a number appended to it) and the use sizeof(name) for the snprintf call.
Cheers
Tobias
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 7/7] staging: cxt1e1: remove unneeded a value
2014-03-06 7:33 ` Tobias Klauser
@ 2014-03-06 7:47 ` DaeSeok Youn
0 siblings, 0 replies; 6+ messages in thread
From: DaeSeok Youn @ 2014-03-06 7:47 UTC (permalink / raw)
To: Tobias Klauser
Cc: Greg KH, devel, Shaun Laing, sachin.kamat, linux-kernel,
Dulshani Gunawardhana, ying.xue, David Miller
Ok.
I will use sizeof(name) for snprintf() call.
Thanks.
Daeseok Youn.
2014-03-06 16:33 GMT+09:00 Tobias Klauser <tklauser@distanz.ch>:
> On 2014-03-06 at 08:19:19 +0100, DaeSeok Youn <daeseok.youn@gmail.com> wrote:
>> 2014-03-05 19:13 GMT+09:00 Tobias Klauser <tklauser@distanz.ch>:
>> > On 2014-03-05 at 02:24:22 +0100, Daeseok Youn <daeseok.youn@gmail.com> wrote:
>> >>
>> >> It doesn't need to assign name array address to np pointer.
>> >>
>> >> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>> >> ---
>> >> drivers/staging/cxt1e1/linux.c | 5 ++---
>> >> 1 files changed, 2 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
>> >> index 5bb42ae..cae8c66 100644
>> >> --- a/drivers/staging/cxt1e1/linux.c
>> >> +++ b/drivers/staging/cxt1e1/linux.c
>> >> @@ -205,15 +205,14 @@ status_t
>> >> c4_wq_port_init(mpi_t *pi)
>> >> {
>> >>
>> >> - char name[16], *np; /* NOTE: name of the queue limited by system
>> >> + char name[16]; /* NOTE: name of the queue limited by system
>> >> * to 10 characters */
>> >>
>> >> if (pi->wq_port)
>> >> return 0; /* already initialized */
>> >>
>> >> - np = name;
>> >> memset(name, 0, 16);
>> >
>> > This isn't necessary since s{,n}printf() adds a terminating '\0'.
>> Yes, I have looked at lib/vsprintf.c. I found it adds null to a string
>> in the end of vsnprintf() function.
>> I will remove memset() line.
>>
>> >
>> >> - sprintf(np, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
>> >> + sprintf(name, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
>> >
>> > Better use snprintf() here, even if the comment above claims the name
>> > never to be no longer than 10 characters.
>> OK. I will replace sprintf with snprintf() and set a string length to "10".
>
> It's probably fine to leave the string at length 16 (since there's also
> a number appended to it) and the use sizeof(name) for the snprintf call.
>
> Cheers
> Tobias
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 7/7] staging: cxt1e1: remove unneeded a value
@ 2014-03-04 2:14 Daeseok Youn
0 siblings, 0 replies; 6+ messages in thread
From: Daeseok Youn @ 2014-03-04 2:14 UTC (permalink / raw)
To: gregkh
Cc: sachin.kamat, shaun, dulshani.gunawardhana89, davem, devel,
linux-kernel, ying.xue
It doesn't need to assign name array address to np pointer.
Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
drivers/staging/cxt1e1/linux.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
index 599a5ef..390b1f5 100644
--- a/drivers/staging/cxt1e1/linux.c
+++ b/drivers/staging/cxt1e1/linux.c
@@ -205,15 +205,14 @@ status_t
c4_wq_port_init(mpi_t *pi)
{
- char name[16], *np; /* NOTE: name of the queue limited by system
+ char name[16]; /* NOTE: name of the queue limited by system
* to 10 characters */
if (pi->wq_port)
return 0; /* already initialized */
- np = name;
memset(name, 0, 16);
- sprintf(np, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
+ sprintf(name, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */
#ifdef RLD_RESTART_DEBUG
pr_info(">> %s: creating workqueue <%s> for Port %d.\n",
--
1.7.4.4
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-06 7:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-05 1:24 [PATCH 7/7] staging: cxt1e1: remove unneeded a value Daeseok Youn
2014-03-05 10:13 ` Tobias Klauser
2014-03-06 7:19 ` DaeSeok Youn
2014-03-06 7:33 ` Tobias Klauser
2014-03-06 7:47 ` DaeSeok Youn
-- strict thread matches above, loose matches on Subject: below --
2014-03-04 2:14 Daeseok Youn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome