Patchwork drivers/tty/hvc: fixup original commit: 9276dfd27897a0b29d8b5814f39a1f82f56b6b6b

login
register
mail settings
Submitter Chen Gang
Date March 8, 2013, 3:38 a.m.
Message ID <51395D2C.6040007@asianux.com>
Download mbox | patch
Permalink /patch/226023/
State Rejected
Headers show

Comments

Chen Gang - March 8, 2013, 3:38 a.m.
originally I did not notice src buf len and dest buf len are the same.
    so origianlly, it is not a bug issue, it is only for beautify code.
    and now, using strcpy is better.


Signed-off-by: Chen Gang <gang.chen@asianux.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/hvc/hvcs.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

 /*
Benjamin Herrenschmidt - March 8, 2013, 3:46 a.m.
On Fri, 2013-03-08 at 11:38 +0800, Chen Gang wrote:
>   originally I did not notice src buf len and dest buf len are the same.
>     so origianlly, it is not a bug issue, it is only for beautify code.
>     and now, using strcpy is better.

Being the same len doesn't mean it's safe to use strcpy ... the source
might be missing the 0 terminator. In this specific case though, I
believe the source string comes was itself populated with strlcpy
(at least since your patch 6b6680c4ea3952af8ae76915cbca41245147741b) so
strcpy is indeed safe but using strlcpy doesn't hurt does it ?

Ben.

> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  drivers/tty/hvc/hvcs.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
> index 81e939e..9330a4b 100644
> --- a/drivers/tty/hvc/hvcs.c
> +++ b/drivers/tty/hvc/hvcs.c
> @@ -885,8 +885,7 @@ static void hvcs_set_pi(struct hvcs_partner_info
> *pi, struct hvcs_struct *hvcsd)
>  	hvcsd->p_partition_ID  = pi->partition_ID;
> 
>  	/* copy the null-term char too */
> -	strlcpy(&hvcsd->p_location_code[0],
> -			&pi->location_code[0], sizeof(hvcsd->p_location_code));
> +	strcpy(&hvcsd->p_location_code[0], &pi->location_code[0]);
>  }
> 
>  /*
Chen Gang - March 8, 2013, 4:23 a.m.
于 2013年03月08日 11:46, Benjamin Herrenschmidt 写道:
> strcpy is indeed safe but using strlcpy doesn't hurt does it ?

  really it is: using strlcpy doesn't hurt.

  the comments and subject of original commit are not quite precision:
    it is not for a bug issue (originally I say it is for bug issue)
    it is really for beautify code.

  can I send a fixup patch only for the comments ?

  thanks.
Benjamin Herrenschmidt - March 8, 2013, 4:33 a.m.
On Fri, 2013-03-08 at 12:23 +0800, Chen Gang wrote:
>   really it is: using strlcpy doesn't hurt.
> 
>   the comments and subject of original commit are not quite precision:
>     it is not for a bug issue (originally I say it is for bug issue)
>     it is really for beautify code.
> 
>   can I send a fixup patch only for the comments ?

No just drop it, it's fine as it is.

Ben.
Chen Gang - March 8, 2013, 4:40 a.m.
于 2013年03月08日 12:33, Benjamin Herrenschmidt 写道:
> On Fri, 2013-03-08 at 12:23 +0800, Chen Gang wrote:
>> >   really it is: using strlcpy doesn't hurt.
>> > 
>> >   the comments and subject of original commit are not quite precision:
>> >     it is not for a bug issue (originally I say it is for bug issue)
>> >     it is really for beautify code.
>> > 
>> >   can I send a fixup patch only for the comments ?
> No just drop it, it's fine as it is.

  I guess your meaning is:
    not need additional fix up patch (just drop it).
    original commit 9276dfd27897a0b29d8b5814f39a1f82f56b6b6b is valid
      (not need drop the original commit)

  is it correct ?

  thanks.
Benjamin Herrenschmidt - March 8, 2013, 5:08 a.m.
On Fri, 2013-03-08 at 12:40 +0800, Chen Gang wrote:
>   is it correct ?

Yes, the code is fine as it is now.

Ben.
Chen Gang - March 8, 2013, 6:12 a.m.
于 2013年03月08日 13:08, Benjamin Herrenschmidt 写道:
> Yes, the code is fine as it is now.

ok, thanks.
David Laight - March 8, 2013, 11:11 a.m.
> On Fri, 2013-03-08 at 11:38 +0800, Chen Gang wrote:
> >   originally I did not notice src buf len and dest buf len are the same.
> >     so origianlly, it is not a bug issue, it is only for beautify code.
> >     and now, using strcpy is better.
> 
> Being the same len doesn't mean it's safe to use strcpy ... the source
> might be missing the 0 terminator. In this specific case though, I
> believe the source string comes was itself populated with strlcpy
> (at least since your patch 6b6680c4ea3952af8ae76915cbca41245147741b) so
> strcpy is indeed safe but using strlcpy doesn't hurt does it ?

Using strlcpy() also stops someone else having to check it
again in a few years time.

	David
Chen Gang - March 10, 2013, 1:18 a.m.
于 2013年03月08日 19:11, David Laight 写道:
> Using strlcpy() also stops someone else having to check it
> again in a few years time.

  yes.

  :-)

Patch

diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index 81e939e..9330a4b 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -885,8 +885,7 @@  static void hvcs_set_pi(struct hvcs_partner_info
*pi, struct hvcs_struct *hvcsd)
 	hvcsd->p_partition_ID  = pi->partition_ID;

 	/* copy the null-term char too */
-	strlcpy(&hvcsd->p_location_code[0],
-			&pi->location_code[0], sizeof(hvcsd->p_location_code));
+	strcpy(&hvcsd->p_location_code[0], &pi->location_code[0]);
 }