netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy
@ 2013-05-07 13:50 Chen Gang
  2013-05-07 14:08 ` [Suggestion] Bluetooth: hidp: redundant initialization or issue for function hidp_copy_session Chen Gang
  2013-05-07 19:31 ` [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy David Herrmann
  0 siblings, 2 replies; 15+ messages in thread
From: Chen Gang @ 2013-05-07 13:50 UTC (permalink / raw)
  To: Marcel Holtmann, gustavo-THi1TnShQwVAfugRpC6u6w,
	johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w
  Cc: David Miller, dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w, Jiri Kosina,
	andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev


For NUL terminated string, need always let it ended by zero.

Since have already called memcpy() to initialize 'ci', so need not
redundent initializations.

Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
---
 net/bluetooth/hidp/core.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 940f5ac..9a8ae63 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -76,25 +76,21 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
 	ci->flags = session->flags;
 	ci->state = BT_CONNECTED;
 
-	ci->vendor  = 0x0000;
-	ci->product = 0x0000;
-	ci->version = 0x0000;
-
 	if (session->input) {
 		ci->vendor  = session->input->id.vendor;
 		ci->product = session->input->id.product;
 		ci->version = session->input->id.version;
 		if (session->input->name)
-			strncpy(ci->name, session->input->name, 128);
+			strlcpy(ci->name, session->input->name, 128);
 		else
-			strncpy(ci->name, "HID Boot Device", 128);
+			strcpy(ci->name, "HID Boot Device");
 	}
 
 	if (session->hid) {
 		ci->vendor  = session->hid->vendor;
 		ci->product = session->hid->product;
 		ci->version = session->hid->version;
-		strncpy(ci->name, session->hid->name, 128);
+		strlcpy(ci->name, session->hid->name, 128);
 	}
 }
 
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Suggestion] Bluetooth: hidp: redundant initialization or issue for function hidp_copy_session
  2013-05-07 13:50 [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy Chen Gang
@ 2013-05-07 14:08 ` Chen Gang
       [not found]   ` <51890AC9.7010501-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
  2013-05-07 19:31 ` [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy David Herrmann
  1 sibling, 1 reply; 15+ messages in thread
From: Chen Gang @ 2013-05-07 14:08 UTC (permalink / raw)
  To: Marcel Holtmann, gustavo, johan.hedberg
  Cc: David Miller, dh.herrmann, Jiri Kosina, andrei.emeltchenko,
	linux-bluetooth, netdev

Hello Maintainers:

In net/bluetooth/hidp/core.c, for hidp_copy_session(), the
'session->input' and 'session->hid' are conflict with each other.

And excuse me, I do not quit know the details, but I think we have 2
choices for fixing it:

  one is ''if (session->input) { } else if (session->hid) { };''
  the other is ''if (seesion->hid) { } else if (session->input) { };''

The first choice assumes the original code has a logical issue; the
second choice assumes the original code has redundant initialization.

Please help check.

Thanks.

  71 static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo *ci)
  72 {
  73         memset(ci, 0, sizeof(*ci));
  74         bacpy(&ci->bdaddr, &session->bdaddr);
  75 
  76         ci->flags = session->flags;
  77         ci->state = BT_CONNECTED;
  78 
  79         ci->vendor  = 0x0000;
  80         ci->product = 0x0000;
  81         ci->version = 0x0000;
  82 
  83         if (session->input) {
  84                 ci->vendor  = session->input->id.vendor;
  85                 ci->product = session->input->id.product;
  86                 ci->version = session->input->id.version;
  87                 if (session->input->name)
  88                         strncpy(ci->name, session->input->name, 128);
  89                 else
  90                         strncpy(ci->name, "HID Boot Device", 128);
  91         }
  92 
  93         if (session->hid) {
  94                 ci->vendor  = session->hid->vendor;
  95                 ci->product = session->hid->product;
  96                 ci->version = session->hid->version;
  97                 strncpy(ci->name, session->hid->name, 128);
  98         }
  99 }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy
  2013-05-07 13:50 [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy Chen Gang
  2013-05-07 14:08 ` [Suggestion] Bluetooth: hidp: redundant initialization or issue for function hidp_copy_session Chen Gang
@ 2013-05-07 19:31 ` David Herrmann
       [not found]   ` <CANq1E4SmY+CD0uf53_b6GeFkQ-LYnpdxsDGSRMWdn2i1mnb6WQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 15+ messages in thread
From: David Herrmann @ 2013-05-07 19:31 UTC (permalink / raw)
  To: Chen Gang
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David Miller,
	Jiri Kosina, andrei.emeltchenko, linux-bluetooth, netdev

Hi

On Tue, May 7, 2013 at 3:50 PM, Chen Gang <gang.chen@asianux.com> wrote:
>
> For NUL terminated string, need always let it ended by zero.
>
> Since have already called memcpy() to initialize 'ci', so need not
> redundent initializations.
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  net/bluetooth/hidp/core.c |   10 +++-------
>  1 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 940f5ac..9a8ae63 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -76,25 +76,21 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
>         ci->flags = session->flags;
>         ci->state = BT_CONNECTED;
>
> -       ci->vendor  = 0x0000;
> -       ci->product = 0x0000;
> -       ci->version = 0x0000;
> -
>         if (session->input) {
>                 ci->vendor  = session->input->id.vendor;
>                 ci->product = session->input->id.product;
>                 ci->version = session->input->id.version;
>                 if (session->input->name)
> -                       strncpy(ci->name, session->input->name, 128);
> +                       strlcpy(ci->name, session->input->name, 128);
>                 else
> -                       strncpy(ci->name, "HID Boot Device", 128);
> +                       strcpy(ci->name, "HID Boot Device");

I'd actually prefer strlcpy() here, too (better be safe). Other than
that the patch looks fine.

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Regards
David

>         }
>
>         if (session->hid) {
>                 ci->vendor  = session->hid->vendor;
>                 ci->product = session->hid->product;
>                 ci->version = session->hid->version;
> -               strncpy(ci->name, session->hid->name, 128);
> +               strlcpy(ci->name, session->hid->name, 128);
>         }
>  }
>
> --
> 1.7.7.6

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Suggestion] Bluetooth: hidp: redundant initialization or issue for function hidp_copy_session
       [not found]   ` <51890AC9.7010501-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
@ 2013-05-07 19:37     ` David Herrmann
       [not found]       ` <CANq1E4Q8g6Uy5DMiEf3aEBmfWN=KnVCfqxErYr=ZWD94D4vmAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: David Herrmann @ 2013-05-07 19:37 UTC (permalink / raw)
  To: Chen Gang
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David Miller,
	Jiri Kosina, andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev

Hi

On Tue, May 7, 2013 at 4:08 PM, Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org> wrote:
> Hello Maintainers:
>
> In net/bluetooth/hidp/core.c, for hidp_copy_session(), the
> 'session->input' and 'session->hid' are conflict with each other.
>
> And excuse me, I do not quit know the details, but I think we have 2
> choices for fixing it:
>
>   one is ''if (session->input) { } else if (session->hid) { };''
>   the other is ''if (seesion->hid) { } else if (session->input) { };''
>
> The first choice assumes the original code has a logical issue; the
> second choice assumes the original code has redundant initialization.

The code is fine. Only one of "->input" or "->hid" can be valid at a
time. And exactly one of them is guaranteed to be valid. See
hidp_session_dev_init().

I fixed all code that I changed during the rework to say:

if (session->hid)
...
else if (session->input)
...

It makes the code more clear. But I avoided touching all the other
places that I didn't change, as the code is technically right. Anyway,
I don't care whether we want to fix all other occurrences to use "else
if". Feel free to send a patch.

Thanks
David

> Please help check.
>
> Thanks.
>
>   71 static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo *ci)
>   72 {
>   73         memset(ci, 0, sizeof(*ci));
>   74         bacpy(&ci->bdaddr, &session->bdaddr);
>   75
>   76         ci->flags = session->flags;
>   77         ci->state = BT_CONNECTED;
>   78
>   79         ci->vendor  = 0x0000;
>   80         ci->product = 0x0000;
>   81         ci->version = 0x0000;
>   82
>   83         if (session->input) {
>   84                 ci->vendor  = session->input->id.vendor;
>   85                 ci->product = session->input->id.product;
>   86                 ci->version = session->input->id.version;
>   87                 if (session->input->name)
>   88                         strncpy(ci->name, session->input->name, 128);
>   89                 else
>   90                         strncpy(ci->name, "HID Boot Device", 128);
>   91         }
>   92
>   93         if (session->hid) {
>   94                 ci->vendor  = session->hid->vendor;
>   95                 ci->product = session->hid->product;
>   96                 ci->version = session->hid->version;
>   97                 strncpy(ci->name, session->hid->name, 128);
>   98         }
>   99 }
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy
       [not found]   ` <CANq1E4SmY+CD0uf53_b6GeFkQ-LYnpdxsDGSRMWdn2i1mnb6WQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2013-05-08  1:02     ` Chen Gang
       [not found]       ` <5189A417.503-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Chen Gang @ 2013-05-08  1:02 UTC (permalink / raw)
  To: David Herrmann
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David Miller,
	Jiri Kosina, andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev

On 2013年05月08日 03:31, David Herrmann wrote:
> On Tue, May 7, 2013 at 3:50 PM, Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org> wrote:
>> >
>> > For NUL terminated string, need always let it ended by zero.
>> >
>> > Since have already called memcpy() to initialize 'ci', so need not
>> > redundent initializations.
>> >
>> > Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
>> > ---
>> >  net/bluetooth/hidp/core.c |   10 +++-------
>> >  1 files changed, 3 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
>> > index 940f5ac..9a8ae63 100644
>> > --- a/net/bluetooth/hidp/core.c
>> > +++ b/net/bluetooth/hidp/core.c
>> > @@ -76,25 +76,21 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
>> >         ci->flags = session->flags;
>> >         ci->state = BT_CONNECTED;
>> >
>> > -       ci->vendor  = 0x0000;
>> > -       ci->product = 0x0000;
>> > -       ci->version = 0x0000;
>> > -
>> >         if (session->input) {
>> >                 ci->vendor  = session->input->id.vendor;
>> >                 ci->product = session->input->id.product;
>> >                 ci->version = session->input->id.version;
>> >                 if (session->input->name)
>> > -                       strncpy(ci->name, session->input->name, 128);
>> > +                       strlcpy(ci->name, session->input->name, 128);
>> >                 else
>> > -                       strncpy(ci->name, "HID Boot Device", 128);
>> > +                       strcpy(ci->name, "HID Boot Device");
> I'd actually prefer strlcpy() here, too (better be safe). Other than
> that the patch looks fine.

OK, thanks. I will send patch v2.

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Suggestion] Bluetooth: hidp: redundant initialization or issue for function hidp_copy_session
       [not found]       ` <CANq1E4Q8g6Uy5DMiEf3aEBmfWN=KnVCfqxErYr=ZWD94D4vmAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2013-05-08  1:50         ` Chen Gang
  0 siblings, 0 replies; 15+ messages in thread
From: Chen Gang @ 2013-05-08  1:50 UTC (permalink / raw)
  To: David Herrmann
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David Miller,
	Jiri Kosina, andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev

On 2013年05月08日 03:37, David Herrmann wrote:
> Hi
> 
> On Tue, May 7, 2013 at 4:08 PM, Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org> wrote:
>> Hello Maintainers:
>>
>> In net/bluetooth/hidp/core.c, for hidp_copy_session(), the
>> 'session->input' and 'session->hid' are conflict with each other.
>>
>> And excuse me, I do not quit know the details, but I think we have 2
>> choices for fixing it:
>>
>>   one is ''if (session->input) { } else if (session->hid) { };''
>>   the other is ''if (seesion->hid) { } else if (session->input) { };''
>>
>> The first choice assumes the original code has a logical issue; the
>> second choice assumes the original code has redundant initialization.
> 
> The code is fine. Only one of "->input" or "->hid" can be valid at a
> time. And exactly one of them is guaranteed to be valid. See
> hidp_session_dev_init().
> 

Oh, really it is, thanks.

> I fixed all code that I changed during the rework to say:
> 
> if (session->hid)
> ..
> else if (session->input)
> ..
> 
> It makes the code more clear. But I avoided touching all the other
> places that I didn't change, as the code is technically right. Anyway,
> I don't care whether we want to fix all other occurrences to use "else
> if". Feel free to send a patch.
> 

Me too: "avoided touching all the other places that I didn't change, as
the code is technically right".

Thanks.

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
       [not found]       ` <5189A417.503-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
@ 2013-05-08  3:34         ` Chen Gang
       [not found]           ` <5189C7C6.8090408-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Chen Gang @ 2013-05-08  3:34 UTC (permalink / raw)
  To: David Herrmann
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David Miller,
	Jiri Kosina, andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev


For NUL terminated string, need always let it ended by zero.

Since have already called memcpy() to initialize 'ci', so need not
redundent initializations.

Better use ''if(session->hid) {} else if(session->input) {}'' instead
of ''if(session->hid) {}; if(session->input) {};''

Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
---
 net/bluetooth/hidp/core.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 940f5ac..f13a8da 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -76,25 +76,19 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
 	ci->flags = session->flags;
 	ci->state = BT_CONNECTED;
 
-	ci->vendor  = 0x0000;
-	ci->product = 0x0000;
-	ci->version = 0x0000;
-
 	if (session->input) {
 		ci->vendor  = session->input->id.vendor;
 		ci->product = session->input->id.product;
 		ci->version = session->input->id.version;
 		if (session->input->name)
-			strncpy(ci->name, session->input->name, 128);
+			strlcpy(ci->name, session->input->name, 128);
 		else
-			strncpy(ci->name, "HID Boot Device", 128);
-	}
-
-	if (session->hid) {
+			strlcpy(ci->name, "HID Boot Device", 128);
+	} else if (session->hid) {
 		ci->vendor  = session->hid->vendor;
 		ci->product = session->hid->product;
 		ci->version = session->hid->version;
-		strncpy(ci->name, session->hid->name, 128);
+		strlcpy(ci->name, session->hid->name, 128);
 	}
 }
 
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
       [not found]           ` <5189C7C6.8090408-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
@ 2013-05-08 15:16             ` David Herrmann
  2013-05-09  1:07               ` Chen Gang
  2013-05-09  8:42             ` Jiri Kosina
  1 sibling, 1 reply; 15+ messages in thread
From: David Herrmann @ 2013-05-08 15:16 UTC (permalink / raw)
  To: Chen Gang
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David Miller,
	Jiri Kosina, andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev

Hi Chen

On Wed, May 8, 2013 at 5:34 AM, Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org> wrote:
>
> For NUL terminated string, need always let it ended by zero.
>
> Since have already called memcpy() to initialize 'ci', so need not
> redundent initializations.
>
> Better use ''if(session->hid) {} else if(session->input) {}'' instead
> of ''if(session->hid) {}; if(session->input) {};''

Yep, looks good now.

Reviewed-by: David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks
David

> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
> ---
>  net/bluetooth/hidp/core.c |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 940f5ac..f13a8da 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -76,25 +76,19 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
>         ci->flags = session->flags;
>         ci->state = BT_CONNECTED;
>
> -       ci->vendor  = 0x0000;
> -       ci->product = 0x0000;
> -       ci->version = 0x0000;
> -
>         if (session->input) {
>                 ci->vendor  = session->input->id.vendor;
>                 ci->product = session->input->id.product;
>                 ci->version = session->input->id.version;
>                 if (session->input->name)
> -                       strncpy(ci->name, session->input->name, 128);
> +                       strlcpy(ci->name, session->input->name, 128);
>                 else
> -                       strncpy(ci->name, "HID Boot Device", 128);
> -       }
> -
> -       if (session->hid) {
> +                       strlcpy(ci->name, "HID Boot Device", 128);
> +       } else if (session->hid) {
>                 ci->vendor  = session->hid->vendor;
>                 ci->product = session->hid->product;
>                 ci->version = session->hid->version;
> -               strncpy(ci->name, session->hid->name, 128);
> +               strlcpy(ci->name, session->hid->name, 128);
>         }
>  }
>
> --
> 1.7.7.6

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
  2013-05-08 15:16             ` David Herrmann
@ 2013-05-09  1:07               ` Chen Gang
  0 siblings, 0 replies; 15+ messages in thread
From: Chen Gang @ 2013-05-09  1:07 UTC (permalink / raw)
  To: David Herrmann
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David Miller,
	Jiri Kosina, andrei.emeltchenko, linux-bluetooth, netdev

On 05/08/2013 11:16 PM, David Herrmann wrote:
> On Wed, May 8, 2013 at 5:34 AM, Chen Gang <gang.chen@asianux.com> wrote:
>> >
>> > For NUL terminated string, need always let it ended by zero.
>> >
>> > Since have already called memcpy() to initialize 'ci', so need not
>> > redundent initializations.
>> >
>> > Better use ''if(session->hid) {} else if(session->input) {}'' instead
>> > of ''if(session->hid) {}; if(session->input) {};''
> Yep, looks good now.
> 
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks.

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
       [not found]           ` <5189C7C6.8090408-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
  2013-05-08 15:16             ` David Herrmann
@ 2013-05-09  8:42             ` Jiri Kosina
       [not found]               ` <alpine.LNX.2.00.1305091041140.23038-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
  2013-05-13  2:07               ` [PATCH v3] " Chen Gang
  1 sibling, 2 replies; 15+ messages in thread
From: Jiri Kosina @ 2013-05-09  8:42 UTC (permalink / raw)
  To: Chen Gang, Gustavo Padovan
  Cc: David Herrmann, Marcel Holtmann, Johan Hedberg, David Miller,
	andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev

On Wed, 8 May 2013, Chen Gang wrote:

> 
> For NUL terminated string, need always let it ended by zero.
> 
> Since have already called memcpy() to initialize 'ci', so need not
> redundent initializations.
> 
> Better use ''if(session->hid) {} else if(session->input) {}'' instead
> of ''if(session->hid) {}; if(session->input) {};''
> 
> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>

Makes sense.

Acked-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>

Gustavo, going to take this, please?

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
       [not found]               ` <alpine.LNX.2.00.1305091041140.23038-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
@ 2013-05-09  8:48                 ` Chen Gang
  0 siblings, 0 replies; 15+ messages in thread
From: Chen Gang @ 2013-05-09  8:48 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Gustavo Padovan, David Herrmann, Marcel Holtmann, Johan Hedberg,
	David Miller, andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev

On 05/09/2013 04:42 PM, Jiri Kosina wrote:
> On Wed, 8 May 2013, Chen Gang wrote:
> 
>> > 
>> > For NUL terminated string, need always let it ended by zero.
>> > 
>> > Since have already called memcpy() to initialize 'ci', so need not
>> > redundent initializations.
>> > 
>> > Better use ''if(session->hid) {} else if(session->input) {}'' instead
>> > of ''if(session->hid) {}; if(session->input) {};''
>> > 
>> > Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
> Makes sense.
> 
> Acked-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>
> 
> Gustavo, going to take this, please?
> 
> -- Jiri Kosina SUSE Labs
> 

Thanks, and also help to fix the spelling: redundent -> redundant.
(if need let me send patch v3, please reply to tell me, thanks)


-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
  2013-05-09  8:42             ` Jiri Kosina
       [not found]               ` <alpine.LNX.2.00.1305091041140.23038-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
@ 2013-05-13  2:07               ` Chen Gang
  2013-05-17  7:04                 ` Chen Gang
       [not found]                 ` <51904ACF.6010904-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
  1 sibling, 2 replies; 15+ messages in thread
From: Chen Gang @ 2013-05-13  2:07 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Gustavo Padovan, David Herrmann, Marcel Holtmann, Johan Hedberg,
	David Miller, andrei.emeltchenko, linux-bluetooth, netdev


For NUL terminated string, need always let it ended by zero.

Since have already called memcpy() to initialize 'ci', so need not
redundant initialization.

Better use ''if(session->hid) {} else if(session->input) {}"" instead
of ''if(session->hid) {}; if(session->input) {};''

Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 net/bluetooth/hidp/core.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 940f5ac..f13a8da 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -76,25 +76,19 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
 	ci->flags = session->flags;
 	ci->state = BT_CONNECTED;
 
-	ci->vendor  = 0x0000;
-	ci->product = 0x0000;
-	ci->version = 0x0000;
-
 	if (session->input) {
 		ci->vendor  = session->input->id.vendor;
 		ci->product = session->input->id.product;
 		ci->version = session->input->id.version;
 		if (session->input->name)
-			strncpy(ci->name, session->input->name, 128);
+			strlcpy(ci->name, session->input->name, 128);
 		else
-			strncpy(ci->name, "HID Boot Device", 128);
-	}
-
-	if (session->hid) {
+			strlcpy(ci->name, "HID Boot Device", 128);
+	} else if (session->hid) {
 		ci->vendor  = session->hid->vendor;
 		ci->product = session->hid->product;
 		ci->version = session->hid->version;
-		strncpy(ci->name, session->hid->name, 128);
+		strlcpy(ci->name, session->hid->name, 128);
 	}
 }
 
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
  2013-05-13  2:07               ` [PATCH v3] " Chen Gang
@ 2013-05-17  7:04                 ` Chen Gang
       [not found]                 ` <51904ACF.6010904-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
  1 sibling, 0 replies; 15+ messages in thread
From: Chen Gang @ 2013-05-17  7:04 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Gustavo Padovan, David Herrmann, Marcel Holtmann, Johan Hedberg,
	David Miller, andrei.emeltchenko, linux-bluetooth, netdev

Hello Maintainers:

Please help check this patch when you have time, thanks.

It is "Reviewed-by: David Herrmann <dh.herrmann@gmail.com>"

And also is "Acked-by: Jiri Kosina <jkosina@suse.cz>"

If need me do any additional things, please tell me.


Thanks.

On 05/13/2013 10:07 AM, Chen Gang wrote:
> 
> For NUL terminated string, need always let it ended by zero.
> 
> Since have already called memcpy() to initialize 'ci', so need not
> redundant initialization.
> 
> Better use ''if(session->hid) {} else if(session->input) {}"" instead
> of ''if(session->hid) {}; if(session->input) {};''
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  net/bluetooth/hidp/core.c |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 940f5ac..f13a8da 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -76,25 +76,19 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
>  	ci->flags = session->flags;
>  	ci->state = BT_CONNECTED;
>  
> -	ci->vendor  = 0x0000;
> -	ci->product = 0x0000;
> -	ci->version = 0x0000;
> -
>  	if (session->input) {
>  		ci->vendor  = session->input->id.vendor;
>  		ci->product = session->input->id.product;
>  		ci->version = session->input->id.version;
>  		if (session->input->name)
> -			strncpy(ci->name, session->input->name, 128);
> +			strlcpy(ci->name, session->input->name, 128);
>  		else
> -			strncpy(ci->name, "HID Boot Device", 128);
> -	}
> -
> -	if (session->hid) {
> +			strlcpy(ci->name, "HID Boot Device", 128);
> +	} else if (session->hid) {
>  		ci->vendor  = session->hid->vendor;
>  		ci->product = session->hid->product;
>  		ci->version = session->hid->version;
> -		strncpy(ci->name, session->hid->name, 128);
> +		strlcpy(ci->name, session->hid->name, 128);
>  	}
>  }
>  
> 


-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
       [not found]                 ` <51904ACF.6010904-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
@ 2013-05-20 21:52                   ` Gustavo Padovan
  2013-05-21  1:40                     ` Chen Gang
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo Padovan @ 2013-05-20 21:52 UTC (permalink / raw)
  To: Chen Gang
  Cc: Jiri Kosina, David Herrmann, Marcel Holtmann, Johan Hedberg,
	David Miller, andrei.emeltchenko-ral2JQCrhuEAvxtiuMwx3w,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev

Hi Chen,

* Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org> [2013-05-13 10:07:11 +0800]:

> 
> For NUL terminated string, need always let it ended by zero.
> 
> Since have already called memcpy() to initialize 'ci', so need not
> redundant initialization.
> 
> Better use ''if(session->hid) {} else if(session->input) {}"" instead
> of ''if(session->hid) {}; if(session->input) {};''
> 
> Signed-off-by: Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
> ---
>  net/bluetooth/hidp/core.c |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)

Sorry for the big delay on this, patches has now been applied to
bluetooth-next. Thanks all.

	Gustavo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code.
  2013-05-20 21:52                   ` Gustavo Padovan
@ 2013-05-21  1:40                     ` Chen Gang
  0 siblings, 0 replies; 15+ messages in thread
From: Chen Gang @ 2013-05-21  1:40 UTC (permalink / raw)
  To: Gustavo Padovan, Jiri Kosina, David Herrmann, Marcel Holtmann,
	Johan Hedberg, David Miller, andrei.emeltchenko, linux-bluetooth,
	netdev

On 05/21/2013 05:52 AM, Gustavo Padovan wrote:
> Hi Chen,
> 
> * Chen Gang <gang.chen@asianux.com> [2013-05-13 10:07:11 +0800]:
> 
>> > 
>> > For NUL terminated string, need always let it ended by zero.
>> > 
>> > Since have already called memcpy() to initialize 'ci', so need not
>> > redundant initialization.
>> > 
>> > Better use ''if(session->hid) {} else if(session->input) {}"" instead
>> > of ''if(session->hid) {}; if(session->input) {};''
>> > 
>> > Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> > ---
>> >  net/bluetooth/hidp/core.c |   14 ++++----------
>> >  1 files changed, 4 insertions(+), 10 deletions(-)
> Sorry for the big delay on this, patches has now been applied to
> bluetooth-next. Thanks all.

It doesn't matter, every member have their own work, especially, most of
us are busy.

Thank you for your work, too.


Thanks.
-- 
Chen Gang

Asianux Corporation

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-05-21  1:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-07 13:50 [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy Chen Gang
2013-05-07 14:08 ` [Suggestion] Bluetooth: hidp: redundant initialization or issue for function hidp_copy_session Chen Gang
     [not found]   ` <51890AC9.7010501-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
2013-05-07 19:37     ` David Herrmann
     [not found]       ` <CANq1E4Q8g6Uy5DMiEf3aEBmfWN=KnVCfqxErYr=ZWD94D4vmAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-05-08  1:50         ` Chen Gang
2013-05-07 19:31 ` [PATCH] Bluetooth: hidp: using strlcpy or strcpy instead of strncpy David Herrmann
     [not found]   ` <CANq1E4SmY+CD0uf53_b6GeFkQ-LYnpdxsDGSRMWdn2i1mnb6WQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-05-08  1:02     ` Chen Gang
     [not found]       ` <5189A417.503-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
2013-05-08  3:34         ` [PATCH v2] Bluetooth: hidp: using strlcpy instead of strncpy, also beautify code Chen Gang
     [not found]           ` <5189C7C6.8090408-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
2013-05-08 15:16             ` David Herrmann
2013-05-09  1:07               ` Chen Gang
2013-05-09  8:42             ` Jiri Kosina
     [not found]               ` <alpine.LNX.2.00.1305091041140.23038-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
2013-05-09  8:48                 ` Chen Gang
2013-05-13  2:07               ` [PATCH v3] " Chen Gang
2013-05-17  7:04                 ` Chen Gang
     [not found]                 ` <51904ACF.6010904-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org>
2013-05-20 21:52                   ` Gustavo Padovan
2013-05-21  1:40                     ` Chen Gang

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).