2013-11-04 01:46:53

by Matthew Monaco

[permalink] [raw]
Subject: [PATCH] tools/hci2hid: properly format device path

From: Matthew Monaco <[email protected]>

---
Hello! I hope this is the correct place to send a bugfix for bluez.
tools/hid2hci.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index bb8a521..8f22047 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -221,18 +221,21 @@ static int usb_switch_dell(int fd, enum mode mode)
static int find_device(struct udev_device *udev_dev)
{
char path[PATH_MAX];
- const char *busnum, *devnum;
+ const char *str;
+ long int busnum, devnum;
int fd;

- busnum = udev_device_get_sysattr_value(udev_dev, "busnum");
- if (!busnum)
+ str = udev_device_get_sysattr_value(udev_dev, "busnum");
+ if (!str)
return -1;
+ busnum = strtol(str, NULL, 0);

- devnum = udev_device_get_sysattr_value(udev_dev, "devnum");
- if (!devnum)
+ str = udev_device_get_sysattr_value(udev_dev, "devnum");
+ if (!str)
return -1;
+ devnum = strtol(str, NULL, 0);

- snprintf(path, sizeof(path), "/dev/bus/usb/%s/%s", busnum, devnum);
+ snprintf(path, sizeof(path), "/dev/bus/usb/%03d/%03d", busnum, devnum);

fd = open(path, O_RDWR, O_CLOEXEC);
if (fd < 0) {
--
1.8.4.2



2013-11-04 16:52:30

by Matthew Monaco

[permalink] [raw]
Subject: Re: [PATCH] tools/hci2hid: properly format device path

Ah, didn't see it in git. Sorry about the noise.

On 11/04/2013 09:42 AM, Vinicius Costa Gomes wrote:
> Hi Bastien,
>
> On 17:23 Mon 04 Nov, Bastien Nocera wrote:
>> On Mon, 2013-11-04 at 14:18 -0200, Vinicius Costa Gomes wrote:
>>> Hi Matthew,
>>>
>>> On 18:46 Sun 03 Nov, Matthew Monaco wrote:
>>>> From: Matthew Monaco <[email protected]>
>>>>
>>>> ---
>>>> Hello! I hope this is the correct place to send a bugfix for bluez.
>>>> tools/hid2hci.c | 15 +++++++++------
>>>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>>
>>> The patch itself looks good.
>>>
>>> What I think is missing is a better description of the problem it solves, I
>>> would suggest including in the commit message the values of 'path' for before
>>> and after the patch, for a situation that the current implementation doesn't
>>> work.
>>>
>>> Changing the subject line to something like: "tools: Fix wrong paths for
>>> adapters" would make it easier to tell that this patch is indeed a bug fix.
>>
>> Isn't this the same patch Giovanni sent a couple of weeks ago?
>>
>> http://thread.gmane.org/gmane.linux.bluez.kernel/38329
>
> Yeah, same fix. Thanks for noticing.
>
>>
>> Cheers
>>
>
>
> Cheers,
>

2013-11-04 16:42:06

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH] tools/hci2hid: properly format device path

Hi Bastien,

On 17:23 Mon 04 Nov, Bastien Nocera wrote:
> On Mon, 2013-11-04 at 14:18 -0200, Vinicius Costa Gomes wrote:
> > Hi Matthew,
> >
> > On 18:46 Sun 03 Nov, Matthew Monaco wrote:
> > > From: Matthew Monaco <[email protected]>
> > >
> > > ---
> > > Hello! I hope this is the correct place to send a bugfix for bluez.
> > > tools/hid2hci.c | 15 +++++++++------
> > > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > The patch itself looks good.
> >
> > What I think is missing is a better description of the problem it solves, I
> > would suggest including in the commit message the values of 'path' for before
> > and after the patch, for a situation that the current implementation doesn't
> > work.
> >
> > Changing the subject line to something like: "tools: Fix wrong paths for
> > adapters" would make it easier to tell that this patch is indeed a bug fix.
>
> Isn't this the same patch Giovanni sent a couple of weeks ago?
>
> http://thread.gmane.org/gmane.linux.bluez.kernel/38329

Yeah, same fix. Thanks for noticing.

>
> Cheers
>


Cheers,
--
Vinicius

2013-11-04 16:23:41

by Bastien Nocera

[permalink] [raw]
Subject: Re: [PATCH] tools/hci2hid: properly format device path

On Mon, 2013-11-04 at 14:18 -0200, Vinicius Costa Gomes wrote:
> Hi Matthew,
>
> On 18:46 Sun 03 Nov, Matthew Monaco wrote:
> > From: Matthew Monaco <[email protected]>
> >
> > ---
> > Hello! I hope this is the correct place to send a bugfix for bluez.
> > tools/hid2hci.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
>
> The patch itself looks good.
>
> What I think is missing is a better description of the problem it solves, I
> would suggest including in the commit message the values of 'path' for before
> and after the patch, for a situation that the current implementation doesn't
> work.
>
> Changing the subject line to something like: "tools: Fix wrong paths for
> adapters" would make it easier to tell that this patch is indeed a bug fix.

Isn't this the same patch Giovanni sent a couple of weeks ago?

http://thread.gmane.org/gmane.linux.bluez.kernel/38329

Cheers


2013-11-04 16:18:05

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH] tools/hci2hid: properly format device path

Hi Matthew,

On 18:46 Sun 03 Nov, Matthew Monaco wrote:
> From: Matthew Monaco <[email protected]>
>
> ---
> Hello! I hope this is the correct place to send a bugfix for bluez.
> tools/hid2hci.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)

The patch itself looks good.

What I think is missing is a better description of the problem it solves, I
would suggest including in the commit message the values of 'path' for before
and after the patch, for a situation that the current implementation doesn't
work.

Changing the subject line to something like: "tools: Fix wrong paths for
adapters" would make it easier to tell that this patch is indeed a bug fix.

>
> diff --git a/tools/hid2hci.c b/tools/hid2hci.c
> index bb8a521..8f22047 100644
> --- a/tools/hid2hci.c
> +++ b/tools/hid2hci.c
> @@ -221,18 +221,21 @@ static int usb_switch_dell(int fd, enum mode mode)
> static int find_device(struct udev_device *udev_dev)
> {
> char path[PATH_MAX];
> - const char *busnum, *devnum;
> + const char *str;
> + long int busnum, devnum;
> int fd;
>
> - busnum = udev_device_get_sysattr_value(udev_dev, "busnum");
> - if (!busnum)
> + str = udev_device_get_sysattr_value(udev_dev, "busnum");
> + if (!str)
> return -1;
> + busnum = strtol(str, NULL, 0);
>
> - devnum = udev_device_get_sysattr_value(udev_dev, "devnum");
> - if (!devnum)
> + str = udev_device_get_sysattr_value(udev_dev, "devnum");
> + if (!str)
> return -1;
> + devnum = strtol(str, NULL, 0);
>
> - snprintf(path, sizeof(path), "/dev/bus/usb/%s/%s", busnum, devnum);
> + snprintf(path, sizeof(path), "/dev/bus/usb/%03d/%03d", busnum, devnum);
>
> fd = open(path, O_RDWR, O_CLOEXEC);
> if (fd < 0) {
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Cheers,
--
Vinicius