[PATCH 1/3] bnep: Calculate ifindex after NULL check

[PATCH 1/3] bnep: Calculate ifindex after NULL check

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 profiles/network/bnep.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 2a74016..0ef85d5 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -518,13 +518,15 @@ static int bnep_add_to_bridge(const char *devname, const char *bridge)
 
 static int bnep_del_from_bridge(const char *devname, const char *bridge)
 {
-	int ifindex = if_nametoindex(devname);
+	int ifindex;
 	struct ifreq ifr;
 	int sk, err;
 
 	if (!devname || !bridge)
 		return -EINVAL;
 
+	ifindex = if_nametoindex(devname);
+
 	sk = socket(AF_INET, SOCK_STREAM, 0);
 	if (sk < 0)
 		return -1;
-- 
1.8.3.2

[PATCH 2/3] android/avdtp: Retry send on EAGAIN as well

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/avdtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/avdtp.c b/android/avdtp.c
index 5da1206..1055bc1 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -444,7 +444,7 @@ static gboolean try_send(int sk, void *data, size_t len)
 
 	do {
 		err = send(sk, data, len, 0);
-	} while (err < 0 && errno == EINTR);
+	} while (err < 0 && (errno == EINTR || errno == EAGAIN));
 
 	if (err < 0) {
 		error("send: %s (%d)", strerror(errno), errno);
-- 
1.8.3.2

[PATCH 3/3] avctp: Fix unchecked return value

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Refactor code so that ioctl() return value is checked.
---
 profiles/audio/avctp.c | 43 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 6669ddc..4d0f906 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1026,27 +1026,52 @@ static int uinput_create(char *name)
 		err = -errno;
 		error("Can't write device information: %s (%d)",
 						strerror(-err), -err);
-		close(fd);
-		return err;
+		goto fail;
+	}
+
+	if (ioctl(fd, UI_SET_EVBIT, EV_KEY) < 0) {
+		err = -errno;
+		error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
+	}
+
+	if (ioctl(fd, UI_SET_EVBIT, EV_REL) < 0) {
+		err = -errno;
+		error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
+	}
+
+	if (ioctl(fd, UI_SET_EVBIT, EV_REP) < 0) {
+		err = -errno;
+		error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
 	}
 
-	ioctl(fd, UI_SET_EVBIT, EV_KEY);
-	ioctl(fd, UI_SET_EVBIT, EV_REL);
-	ioctl(fd, UI_SET_EVBIT, EV_REP);
-	ioctl(fd, UI_SET_EVBIT, EV_SYN);
+	if (ioctl(fd, UI_SET_EVBIT, EV_SYN) < 0) {
+		err = -errno;
+		error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
+	}
 
-	for (i = 0; key_map[i].name != NULL; i++)
+	for (i = 0; key_map[i].name != NULL; i++) {
 		ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput);
+		err = -errno;
+		error("ioctl UI_SET_KEYBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
+	}
 
 	if (ioctl(fd, UI_DEV_CREATE, NULL) < 0) {
 		err = -errno;
 		error("Can't create uinput device: %s (%d)",
 						strerror(-err), -err);
-		close(fd);
-		return err;
+		goto fail;
 	}
 
 	return fd;
+
+fail:
+	close(fd);
+	return err;
 }
 
 static void init_uinput(struct avctp *session)
-- 
1.8.3.2

Re: [PATCH 3/3] avctp: Fix unchecked return value

[Anderson Lizardo]

Hi Andrei,

On Fri, Jan 24, 2014 at 10:56 AM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:

> -       for (i = 0; key_map[i].name != NULL; i++)
> +       for (i = 0; key_map[i].name != NULL; i++) {
>                 ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput);
> +               err = -errno;
> +               error("ioctl UI_SET_KEYBIT: %s (%d)", strerror(-err), -err);
> +               goto fail;
> +       }

You seem to have missed the "if (ioctl(...) < 0)" above.

Best Regards,
-- 
Anderson Lizardo
http://www.indt.org/?lang=en
INdT - Manaus - Brazil

Re: [PATCH 2/3] android/avdtp: Retry send on EAGAIN as well

[Szymon Janc]

Hi Andrei,

On Friday 24 January 2014 16:56:42 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> ---
>  android/avdtp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/android/avdtp.c b/android/avdtp.c
> index 5da1206..1055bc1 100644
> --- a/android/avdtp.c
> +++ b/android/avdtp.c
> @@ -444,7 +444,7 @@ static gboolean try_send(int sk, void *data, size_t len)
> 
>  	do {
>  		err = send(sk, data, len, 0);
> -	} while (err < 0 && errno == EINTR);
> +	} while (err < 0 && (errno == EINTR || errno == EAGAIN));
> 

This makes try_send() blocking, if this is ok then at least some explanation 
in commit message would be welcome.

>  	if (err < 0) {
>  		error("send: %s (%d)", strerror(errno), errno);

-- 
Szymon K. Janc
szymon.janc@gmail.com

Re: [PATCH 2/3] android/avdtp: Retry send on EAGAIN as well

[Luiz Augusto von Dentz]

Hi Szymon,

On Sat, Jan 25, 2014 at 12:16 AM, Szymon Janc <szymon.janc@gmail.com> wrote:
> Hi Andrei,
>
> On Friday 24 January 2014 16:56:42 Andrei Emeltchenko wrote:
>> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>>
>> ---
>>  android/avdtp.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/android/avdtp.c b/android/avdtp.c
>> index 5da1206..1055bc1 100644
>> --- a/android/avdtp.c
>> +++ b/android/avdtp.c
>> @@ -444,7 +444,7 @@ static gboolean try_send(int sk, void *data, size_t len)
>>
>>       do {
>>               err = send(sk, data, len, 0);
>> -     } while (err < 0 && errno == EINTR);
>> +     } while (err < 0 && (errno == EINTR || errno == EAGAIN));
>>
>
> This makes try_send() blocking, if this is ok then at least some explanation
> in commit message would be welcome.

And if we want to block we should not set the socket as non-blocking
in first place, Im not even sure this is easy to currently reproduce
any case that would block since the command are quite small we would
have to schedule several in a sequence but normally we can only have
one command outstanding.


-- 
Luiz Augusto von Dentz

[PATCHv2] avctp: Fix unchecked return value

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Refactor code so that ioctl() return value is checked.
---
 profiles/audio/avctp.c | 47 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 6fd1454..bf44f9c 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1027,27 +1027,54 @@ static int uinput_create(char *name)
 		err = -errno;
 		error("Can't write device information: %s (%d)",
 						strerror(-err), -err);
-		close(fd);
-		return err;
+		goto fail;
+	}
+
+	if (ioctl(fd, UI_SET_EVBIT, EV_KEY) < 0) {
+		err = -errno;
+		error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
 	}
 
-	ioctl(fd, UI_SET_EVBIT, EV_KEY);
-	ioctl(fd, UI_SET_EVBIT, EV_REL);
-	ioctl(fd, UI_SET_EVBIT, EV_REP);
-	ioctl(fd, UI_SET_EVBIT, EV_SYN);
+	if (ioctl(fd, UI_SET_EVBIT, EV_REL) < 0) {
+		err = -errno;
+		error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
+	}
 
-	for (i = 0; key_map[i].name != NULL; i++)
-		ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput);
+	if (ioctl(fd, UI_SET_EVBIT, EV_REP) < 0) {
+		err = -errno;
+		error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
+	}
+
+	if (ioctl(fd, UI_SET_EVBIT, EV_SYN) < 0) {
+		err = -errno;
+		error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
+		goto fail;
+	}
+
+	for (i = 0; key_map[i].name != NULL; i++) {
+		if (ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput) < 0) {
+			err = -errno;
+			error("ioctl UI_SET_KEYBIT: %s (%d)", strerror(-err),
+									-err);
+			goto fail;
+		}
+	}
 
 	if (ioctl(fd, UI_DEV_CREATE, NULL) < 0) {
 		err = -errno;
 		error("Can't create uinput device: %s (%d)",
 						strerror(-err), -err);
-		close(fd);
-		return err;
+		goto fail;
 	}
 
 	return fd;
+
+fail:
+	close(fd);
+	return err;
 }
 
 static void init_uinput(struct avctp *session)
-- 
1.8.3.2

Re: [PATCHv2] avctp: Fix unchecked return value

[Luiz Augusto von Dentz]

Hi Andrei,

On Mon, Jan 27, 2014 at 7:59 AM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Refactor code so that ioctl() return value is checked.
> ---
>  profiles/audio/avctp.c | 47 +++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> index 6fd1454..bf44f9c 100644
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -1027,27 +1027,54 @@ static int uinput_create(char *name)
>                 err = -errno;
>                 error("Can't write device information: %s (%d)",
>                                                 strerror(-err), -err);
> -               close(fd);
> -               return err;
> +               goto fail;
> +       }
> +
> +       if (ioctl(fd, UI_SET_EVBIT, EV_KEY) < 0) {
> +               err = -errno;
> +               error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
> +               goto fail;
>         }
>
> -       ioctl(fd, UI_SET_EVBIT, EV_KEY);
> -       ioctl(fd, UI_SET_EVBIT, EV_REL);
> -       ioctl(fd, UI_SET_EVBIT, EV_REP);
> -       ioctl(fd, UI_SET_EVBIT, EV_SYN);
> +       if (ioctl(fd, UI_SET_EVBIT, EV_REL) < 0) {
> +               err = -errno;
> +               error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
> +               goto fail;
> +       }
>
> -       for (i = 0; key_map[i].name != NULL; i++)
> -               ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput);
> +       if (ioctl(fd, UI_SET_EVBIT, EV_REP) < 0) {
> +               err = -errno;
> +               error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
> +               goto fail;
> +       }
> +
> +       if (ioctl(fd, UI_SET_EVBIT, EV_SYN) < 0) {
> +               err = -errno;
> +               error("ioctl UI_SET_EVBIT: %s (%d)", strerror(-err), -err);
> +               goto fail;
> +       }
> +
> +       for (i = 0; key_map[i].name != NULL; i++) {
> +               if (ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput) < 0) {
> +                       err = -errno;
> +                       error("ioctl UI_SET_KEYBIT: %s (%d)", strerror(-err),
> +                                                                       -err);
> +                       goto fail;
> +               }
> +       }
>
>         if (ioctl(fd, UI_DEV_CREATE, NULL) < 0) {
>                 err = -errno;
>                 error("Can't create uinput device: %s (%d)",
>                                                 strerror(-err), -err);
> -               close(fd);
> -               return err;
> +               goto fail;
>         }
>
>         return fd;
> +
> +fail:
> +       close(fd);
> +       return err;
>  }
>
>  static void init_uinput(struct avctp *session)
> --
> 1.8.3.2

I start to wonder if there is a better way to set those bits...


-- 
Luiz Augusto von Dentz