[PATCH RFC BlueZ] serial: Fix bogus errno usage

[PATCH RFC BlueZ] serial: Fix bogus errno usage

[Anderson Lizardo]

There is no previous function setting errno, thus the original code may
return a undefined value.
---

NOTE:
Is bt_io_connect() supposed to set errno on error? If yes, then the code is
correct and this patch is invalid.

But in this case, bt_io_connect() needs to be modified as there are at least
two glib library calls before returning, and this may reset errno from the
underlying connect call. So I think it needs to be saved before calling these
library functions.


 serial/port.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/serial/port.c b/serial/port.c
index 3b36d44..1ceda7b 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -463,10 +463,8 @@ connect:
 				BT_IO_OPT_DEST_BDADDR, &device->dst,
 				BT_IO_OPT_CHANNEL, port->channel,
 				BT_IO_OPT_INVALID);
-	if (port->io)
-		return 0;
 
-	return -errno;
+	return 0;
 }
 
 static struct serial_port *create_port(struct serial_device *device,
-- 
1.7.0.4

Re: [PATCH RFC BlueZ] serial: Fix bogus errno usage

[Johan Hedberg]

Hi Lizardo,

On Thu, Nov 17, 2011, Anderson Lizardo wrote:
> There is no previous function setting errno, thus the original code may
> return a undefined value.
> ---
> NOTE:
> Is bt_io_connect() supposed to set errno on error? If yes, then the code is
> correct and this patch is invalid.
> 
> But in this case, bt_io_connect() needs to be modified as there are at least
> two glib library calls before returning, and this may reset errno from the
> underlying connect call. So I think it needs to be saved before calling these
> library functions.
> 
>  serial/port.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/serial/port.c b/serial/port.c
> index 3b36d44..1ceda7b 100644
> --- a/serial/port.c
> +++ b/serial/port.c
> @@ -463,10 +463,8 @@ connect:
>  				BT_IO_OPT_DEST_BDADDR, &device->dst,
>  				BT_IO_OPT_CHANNEL, port->channel,
>  				BT_IO_OPT_INVALID);
> -	if (port->io)
> -		return 0;
>  
> -	return -errno;
> +	return 0;
>  }

bt_io_connect is not supposed to touch errno, so please don't send
patches for that. If you want to get the exact error you pass a GError
pointer, however even without that it's valid to check at least for a
NULL return. A simple fix if you don't want to add GError to this port.c
function is to return e.g. -EIO in case bt_io_connect returns NULL.

Johan

Re: [PATCH RFC BlueZ] serial: Fix bogus errno usage

[Anderson Lizardo]

Hi Johan,

On Fri, Nov 18, 2011 at 7:27 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> bt_io_connect is not supposed to touch errno, so please don't send
> patches for that. If you want to get the exact error you pass a GError
> pointer, however even without that it's valid to check at least for a
> NULL return. A simple fix if you don't want to add GError to this port.c
> function is to return e.g. -EIO in case bt_io_connect returns NULL.

I just sent another patch that uses GError if it is set. I hope this
way it works just like the bt_io_connect() call in get_record_cb(), on
the same file.

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil