From: Andrei Emeltchenko <[email protected]>
AVCTP issues found during writing test cases.
Changes:
* v2: Added last 2 patches to series.
Andrei Emeltchenko (8):
android/avctp: Use predefined HEADER_LENGTH instead of sizeof
android/avctp: Fix wrong error message
avctp: Use already calculated avc header
avctp: trivial: Do not break short line
avctp: Use predefined HEADER_LENGTH instead of sizeof
avctp: Fix wrong error message
android/avctp: Move AVC_HEADER_LENGTH from avctp header
avctp: Fix unchecked return value
android/avctp.c | 16 +++++++------
android/avctp.h | 1 -
profiles/audio/avctp.c | 64 +++++++++++++++++++++++++++++++++++---------------
3 files changed, 54 insertions(+), 27 deletions(-)
--
1.8.3.2
Hi Andrei,
On Wed, Jan 29, 2014 at 4:35 AM, Andrei Emeltchenko
<[email protected]> wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> AVCTP issues found during writing test cases.
>
> Changes:
> * v2: Added last 2 patches to series.
>
> Andrei Emeltchenko (8):
> android/avctp: Use predefined HEADER_LENGTH instead of sizeof
> android/avctp: Fix wrong error message
> avctp: Use already calculated avc header
> avctp: trivial: Do not break short line
> avctp: Use predefined HEADER_LENGTH instead of sizeof
> avctp: Fix wrong error message
> android/avctp: Move AVC_HEADER_LENGTH from avctp header
> avctp: Fix unchecked return value
>
> android/avctp.c | 16 +++++++------
> android/avctp.h | 1 -
> profiles/audio/avctp.c | 64 +++++++++++++++++++++++++++++++++++---------------
> 3 files changed, 54 insertions(+), 27 deletions(-)
>
> --
> 1.8.3.2
Patches 1-7 applied, patch 8 needs some investigation if we can reduce
that many ioctl in a row.
--
Luiz Augusto von Dentz
From: Andrei Emeltchenko <[email protected]>
---
android/avctp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/avctp.c b/android/avctp.c
index 4ebf2ff..d308ec1 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -827,7 +827,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
ret -= AVCTP_HEADER_LENGTH;
if (ret < AVC_HEADER_LENGTH) {
- error("Too small AVCTP packet");
+ error("Too small AVC packet");
goto failed;
}
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
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 9f87859..a77aa9d 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1026,27 +1026,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
From: Andrei Emeltchenko <[email protected]>
Make code consistent with using HEADER_LENGTH defined. Remove also type
conversion.
---
android/avctp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/android/avctp.c b/android/avctp.c
index 6f9b33b..4ebf2ff 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -818,24 +818,24 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
if (ret <= 0)
goto failed;
- if ((unsigned int) ret < sizeof(struct avctp_header)) {
+ if (ret < AVCTP_HEADER_LENGTH) {
error("Too small AVCTP packet");
goto failed;
}
avctp = (struct avctp_header *) buf;
- ret -= sizeof(struct avctp_header);
- if ((unsigned int) ret < sizeof(struct avc_header)) {
+ ret -= AVCTP_HEADER_LENGTH;
+ if (ret < AVC_HEADER_LENGTH) {
error("Too small AVCTP packet");
goto failed;
}
- avc = (struct avc_header *) (buf + sizeof(struct avctp_header));
+ avc = (struct avc_header *) (buf + AVCTP_HEADER_LENGTH);
- ret -= sizeof(struct avc_header);
+ ret -= AVC_HEADER_LENGTH;
- operands = (uint8_t *) avc + sizeof(struct avc_header);
+ operands = (uint8_t *) avc + AVC_HEADER_LENGTH;
operand_count = ret;
if (avctp->cr == AVCTP_RESPONSE) {
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
Make code consistent with using HEADER_LENGTH already defined and used
instead of sizeof(). Remove also ugly type conversion.
---
profiles/audio/avctp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 570d609..0a1a457 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -924,24 +924,24 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
if (ret <= 0)
goto failed;
- if ((unsigned int) ret < sizeof(struct avctp_header)) {
+ if (ret < AVCTP_HEADER_LENGTH) {
error("Too small AVCTP packet");
goto failed;
}
avctp = (struct avctp_header *) buf;
- ret -= sizeof(struct avctp_header);
- if ((unsigned int) ret < sizeof(struct avc_header)) {
+ ret -= AVCTP_HEADER_LENGTH;
+ if (ret < AVC_HEADER_LENGTH) {
error("Too small AVCTP packet");
goto failed;
}
- avc = (struct avc_header *) (buf + sizeof(struct avctp_header));
+ avc = (struct avc_header *) (buf + AVCTP_HEADER_LENGTH);
- ret -= sizeof(struct avc_header);
+ ret -= AVC_HEADER_LENGTH;
- operands = (uint8_t *) avc + sizeof(struct avc_header);
+ operands = (uint8_t *) avc + AVC_HEADER_LENGTH;
operand_count = ret;
if (avctp->cr == AVCTP_RESPONSE) {
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
profiles/audio/avctp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 0a1a457..9f87859 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -933,7 +933,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
ret -= AVCTP_HEADER_LENGTH;
if (ret < AVC_HEADER_LENGTH) {
- error("Too small AVCTP packet");
+ error("Too small AVC packet");
goto failed;
}
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
profiles/audio/avctp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 78bf1fd..570d609 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -904,8 +904,7 @@ failed:
return FALSE;
}
-static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
- gpointer data)
+static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
{
struct avctp *session = data;
struct avctp_channel *control = session->control;
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
This removes long (81 characters) line as a side effect.
---
profiles/audio/avctp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 6fd1454..78bf1fd 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -942,7 +942,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
ret -= sizeof(struct avc_header);
- operands = buf + sizeof(struct avctp_header) + sizeof(struct avc_header);
+ operands = (uint8_t *) avc + sizeof(struct avc_header);
operand_count = ret;
if (avctp->cr == AVCTP_RESPONSE) {
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
There is currently inconsistence in the avctp code with
AVC_HEADER_LENGTH defined in avctp.h but AVCTP_HEADER_LENGTH defined in
avctp.c. Move definition to place it is actually used in consistent way.
---
android/avctp.c | 2 ++
android/avctp.h | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/android/avctp.c b/android/avctp.c
index d308ec1..8f35403 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -84,6 +84,7 @@ struct avc_header {
uint8_t subunit_type:5;
uint8_t opcode;
} __attribute__ ((packed));
+#define AVC_HEADER_LENGTH 3
#elif __BYTE_ORDER == __BIG_ENDIAN
@@ -103,6 +104,7 @@ struct avc_header {
uint8_t subunit_id:3;
uint8_t opcode;
} __attribute__ ((packed));
+#define AVC_HEADER_LENGTH 3
#else
#error "Unknown byte order"
diff --git a/android/avctp.h b/android/avctp.h
index a22bf13..e7e0277 100644
--- a/android/avctp.h
+++ b/android/avctp.h
@@ -26,7 +26,6 @@
#define AVCTP_BROWSING_PSM 27
#define AVC_MTU 512
-#define AVC_HEADER_LENGTH 3
/* ctype entries */
#define AVC_CTYPE_CONTROL 0x0
--
1.8.3.2