| Message ID | 512C2F5D.1080207@asianux.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 9276dfd27897a0b29d8b5814f39a1f82f56b6b6b |
| Headers | show |
On 02/26/2013 04:43 AM, Chen Gang wrote: > > when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1, > original implementation can not let hvcsd->p_location_code NUL terminated. > so need fix it (also can simplify the code) It should never be larger because the +1 is exactly for NUL. But it is a cleanup, so why not... > Signed-off-by: Chen Gang <gang.chen@asianux.com> > --- > drivers/tty/hvc/hvcs.c | 9 ++------- > 1 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c > index 1956593..81e939e 100644 > --- a/drivers/tty/hvc/hvcs.c > +++ b/drivers/tty/hvc/hvcs.c > @@ -881,17 +881,12 @@ static struct vio_driver hvcs_vio_driver = { > /* Only called from hvcs_get_pi please */ > static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct *hvcsd) > { > - int clclength; > - > hvcsd->p_unit_address = pi->unit_address; > hvcsd->p_partition_ID = pi->partition_ID; > - clclength = strlen(&pi->location_code[0]); > - if (clclength > HVCS_CLC_LENGTH) > - clclength = HVCS_CLC_LENGTH; > > /* copy the null-term char too */ > - strncpy(&hvcsd->p_location_code[0], > - &pi->location_code[0], clclength + 1); > + strlcpy(&hvcsd->p_location_code[0], > + &pi->location_code[0], sizeof(hvcsd->p_location_code)); > } > > /* >
于 2013年02月28日 18:41, Jiri Slaby 写道: > On 02/26/2013 04:43 AM, Chen Gang wrote: >> > >> > when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1, >> > original implementation can not let hvcsd->p_location_code NUL terminated. >> > so need fix it (also can simplify the code) > It should never be larger because the +1 is exactly for NUL. But it is a > cleanup, so why not... > when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2 then clclength will be reset to HVCS_CLC_LENGTH. when call strncpy, the clclength + 1 == HVCS_CLS_LENGTH + 1 but the '\0' of src buf is located at HVCS_CLS_LENGTH + 2. so no '\0' copied to dest buf. then the dest buf will not be ended by '\0'. is it correct ? :-) gchen. >> > Signed-off-by: Chen Gang <gang.chen@asianux.com> >> > --- >> > drivers/tty/hvc/hvcs.c | 9 ++------- >> > 1 files changed, 2 insertions(+), 7 deletions(-) >> > >> > diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c >> > index 1956593..81e939e 100644 >> > --- a/drivers/tty/hvc/hvcs.c >> > +++ b/drivers/tty/hvc/hvcs.c >> > @@ -881,17 +881,12 @@ static struct vio_driver hvcs_vio_driver = { >> > /* Only called from hvcs_get_pi please */ >> > static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct *hvcsd) >> > { >> > - int clclength; >> > - >> > hvcsd->p_unit_address = pi->unit_address; >> > hvcsd->p_partition_ID = pi->partition_ID; >> > - clclength = strlen(&pi->location_code[0]); >> > - if (clclength > HVCS_CLC_LENGTH) >> > - clclength = HVCS_CLC_LENGTH; >> > >> > /* copy the null-term char too */ >> > - strncpy(&hvcsd->p_location_code[0], >> > - &pi->location_code[0], clclength + 1); >> > + strlcpy(&hvcsd->p_location_code[0], >> > + &pi->location_code[0], sizeof(hvcsd->p_location_code)); >> > } >> > >> > /* >> >
于 2013年02月28日 19:13, Chen Gang 写道: > 于 2013年02月28日 18:41, Jiri Slaby 写道: >> On 02/26/2013 04:43 AM, Chen Gang wrote: >>>> >>>> when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1, >>>> original implementation can not let hvcsd->p_location_code NUL terminated. >>>> so need fix it (also can simplify the code) >> It should never be larger because the +1 is exactly for NUL. But it is a >> cleanup, so why not... >> > > when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2 > then clclength will be reset to HVCS_CLC_LENGTH. > > when call strncpy, the clclength + 1 == HVCS_CLS_LENGTH + 1 > but the '\0' of src buf is located at HVCS_CLS_LENGTH + 2. but the '\0' of src buf is located at HVCS_CLS_LENGTH + 3. (not + 2) > so no '\0' copied to dest buf. > > then the dest buf will not be ended by '\0'. > > is it correct ? > > :-) > > gchen. > >>>> Signed-off-by: Chen Gang <gang.chen@asianux.com> >>>> --- >>>> drivers/tty/hvc/hvcs.c | 9 ++------- >>>> 1 files changed, 2 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c >>>> index 1956593..81e939e 100644 >>>> --- a/drivers/tty/hvc/hvcs.c >>>> +++ b/drivers/tty/hvc/hvcs.c >>>> @@ -881,17 +881,12 @@ static struct vio_driver hvcs_vio_driver = { >>>> /* Only called from hvcs_get_pi please */ >>>> static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct *hvcsd) >>>> { >>>> - int clclength; >>>> - >>>> hvcsd->p_unit_address = pi->unit_address; >>>> hvcsd->p_partition_ID = pi->partition_ID; >>>> - clclength = strlen(&pi->location_code[0]); >>>> - if (clclength > HVCS_CLC_LENGTH) >>>> - clclength = HVCS_CLC_LENGTH; >>>> >>>> /* copy the null-term char too */ >>>> - strncpy(&hvcsd->p_location_code[0], >>>> - &pi->location_code[0], clclength + 1); >>>> + strlcpy(&hvcsd->p_location_code[0], >>>> + &pi->location_code[0], sizeof(hvcsd->p_location_code)); >>>> } >>>> >>>> /* >>>> > >
On 02/28/2013 12:15 PM, Chen Gang wrote: > 于 2013年02月28日 19:13, Chen Gang 写道: >> 于 2013年02月28日 18:41, Jiri Slaby 写道: >>> On 02/26/2013 04:43 AM, Chen Gang wrote: >>>>> >>>>> when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1, >>>>> original implementation can not let hvcsd->p_location_code NUL terminated. >>>>> so need fix it (also can simplify the code) >>> It should never be larger because the +1 is exactly for NUL. But it is a >>> cleanup, so why not... >>> >> >> when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2 It cannot, pi->location_code is defined as char[HVCS_CLC_LENGTH + 1].
于 2013年02月28日 21:47, Jiri Slaby 写道: >>> when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2 > It cannot, pi->location_code is defined as char[HVCS_CLC_LENGTH + 1]. > really, it is, I did not notice it. but I still prefer to modify it, but the patch should be changed such as: subject: beautify code: deleting useless judging code. comments: src buf len and dest buf len are the same, strcpy is better. contents: using strcpy instead of strncpy, and delete judging code. is it ok ? BTW: sorry for my reply is too late, and did not notify it, originally before. I have to do some urgent things, during these days. my father had a serious heart disease, and is in hospital. during these days, most of my time has to be in hospital. (God Bless, and thank Jesus Christ, my father is safe, now). within my company (Asianux), I also have something to do. excuse me: within this week, maybe can not get my mail reply, too.
On 03/05/2013 02:58 AM, Chen Gang wrote: > 于 2013年02月28日 21:47, Jiri Slaby 写道: >>>> when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2 >> It cannot, pi->location_code is defined as char[HVCS_CLC_LENGTH + 1]. >> > > really, it is, I did not notice it. > > but I still prefer to modify it, but the patch should be changed > such as: > subject: beautify code: deleting useless judging code. > comments: src buf len and dest buf len are the same, strcpy is better. > contents: using strcpy instead of strncpy, and delete judging code. > > is it ok ? Yeah. > BTW: > sorry for my reply is too late, and did not notify it, originally before. > I have to do some urgent things, during these days. > my father had a serious heart disease, and is in hospital. No problem, these drivers are not so critical. Neither these code paths in them. Take care of your relatives first.
于 2013年03月05日 17:36, Jiri Slaby 写道: > On 03/05/2013 02:58 AM, Chen Gang wrote: >> > 于 2013年02月28日 21:47, Jiri Slaby 写道: >>>>> >>>> when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2 >>> >> It cannot, pi->location_code is defined as char[HVCS_CLC_LENGTH + 1]. >>> >> >> > >> > really, it is, I did not notice it. >> > >> > but I still prefer to modify it, but the patch should be changed >> > such as: >> > subject: beautify code: deleting useless judging code. >> > comments: src buf len and dest buf len are the same, strcpy is better. >> > contents: using strcpy instead of strncpy, and delete judging code. >> > >> > is it ok ? > Yeah. > I will send patch v2. >> > BTW: >> > sorry for my reply is too late, and did not notify it, originally before. >> > I have to do some urgent things, during these days. >> > my father had a serious heart disease, and is in hospital. > No problem, these drivers are not so critical. Neither these code paths > in them. Take care of your relatives first. thanks.
oh, this patch has integrated into next-20130307 tree.
(commit 9276dfd27897a0b29d8b5814f39a1f82f56b6b6b)
it seems we need a regression for this commit, then I send patch v2
is it correct ?
:-)
于 2013年03月07日 12:10, Chen Gang 写道:
> 于 2013年03月05日 17:36, Jiri Slaby 写道:
>> On 03/05/2013 02:58 AM, Chen Gang wrote:
>>>> 于 2013年02月28日 21:47, Jiri Slaby 写道:
>>>>>>>>>> when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2
>>>>>> It cannot, pi->location_code is defined as char[HVCS_CLC_LENGTH + 1].
>>>>>>
>>>>
>>>> really, it is, I did not notice it.
>>>>
>>>> but I still prefer to modify it, but the patch should be changed
>>>> such as:
>>>> subject: beautify code: deleting useless judging code.
>>>> comments: src buf len and dest buf len are the same, strcpy is better.
>>>> contents: using strcpy instead of strncpy, and delete judging code.
>>>>
>>>> is it ok ?
>> Yeah.
>>
>
> I will send patch v2.
>
>
>>>> BTW:
>>>> sorry for my reply is too late, and did not notify it, originally before.
>>>> I have to do some urgent things, during these days.
>>>> my father had a serious heart disease, and is in hospital.
>> No problem, these drivers are not so critical. Neither these code paths
>> in them. Take care of your relatives first.
>
> thanks.
>
On Thu, 2013-03-07 at 12:34 +0800, Chen Gang wrote: > oh, this patch has integrated into next-20130307 tree. > (commit 9276dfd27897a0b29d8b5814f39a1f82f56b6b6b) > it seems we need a regression for this commit, then I send patch v2 > > is it correct ? Just send a fixup patch on top of the existing upstream. Ben. > :-) > > > 于 2013年03月07日 12:10, Chen Gang 写道: > > 于 2013年03月05日 17:36, Jiri Slaby 写道: > >> On 03/05/2013 02:58 AM, Chen Gang wrote: > >>>> 于 2013年02月28日 21:47, Jiri Slaby 写道: > >>>>>>>>>> when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2 > >>>>>> It cannot, pi->location_code is defined as char[HVCS_CLC_LENGTH + 1]. > >>>>>> > >>>> > >>>> really, it is, I did not notice it. > >>>> > >>>> but I still prefer to modify it, but the patch should be changed > >>>> such as: > >>>> subject: beautify code: deleting useless judging code. > >>>> comments: src buf len and dest buf len are the same, strcpy is better. > >>>> contents: using strcpy instead of strncpy, and delete judging code. > >>>> > >>>> is it ok ? > >> Yeah. > >> > > > > I will send patch v2. > > > > > >>>> BTW: > >>>> sorry for my reply is too late, and did not notify it, originally before. > >>>> I have to do some urgent things, during these days. > >>>> my father had a serious heart disease, and is in hospital. > >> No problem, these drivers are not so critical. Neither these code paths > >> in them. Take care of your relatives first. > > > > thanks. > > > >
于 2013年03月07日 14:05, Benjamin Herrenschmidt 写道: > On Thu, 2013-03-07 at 12:34 +0800, Chen Gang wrote: >> > oh, this patch has integrated into next-20130307 tree. >> > (commit 9276dfd27897a0b29d8b5814f39a1f82f56b6b6b) >> > it seems we need a regression for this commit, then I send patch v2 >> > >> > is it correct ? > Just send a fixup patch on top of the existing upstream. ok, thanks. :-)
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c index 1956593..81e939e 100644 --- a/drivers/tty/hvc/hvcs.c +++ b/drivers/tty/hvc/hvcs.c @@ -881,17 +881,12 @@ static struct vio_driver hvcs_vio_driver = { /* Only called from hvcs_get_pi please */ static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct *hvcsd) { - int clclength; - hvcsd->p_unit_address = pi->unit_address; hvcsd->p_partition_ID = pi->partition_ID; - clclength = strlen(&pi->location_code[0]); - if (clclength > HVCS_CLC_LENGTH) - clclength = HVCS_CLC_LENGTH; /* copy the null-term char too */ - strncpy(&hvcsd->p_location_code[0], - &pi->location_code[0], clclength + 1); + strlcpy(&hvcsd->p_location_code[0], + &pi->location_code[0], sizeof(hvcsd->p_location_code)); } /*
when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1, original implementation can not let hvcsd->p_location_code NUL terminated. so need fix it (also can simplify the code) Signed-off-by: Chen Gang <gang.chen@asianux.com> --- drivers/tty/hvc/hvcs.c | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-)