2014-01-28 12:34:45

by Andrei Emeltchenko

Subject: [RFC] android/avctp: Move struct definitions to header

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. The patch moves structure definitions to the header in
consistent way.
---
android/avctp.c | 42 ------------------------------------------
android/avctp.h | 45 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/android/avctp.c b/android/avctp.c
index af2046a..7aa0673 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -66,48 +66,6 @@
#define AVCTP_PACKET_CONTINUE 2
#define AVCTP_PACKET_END 3

-#if __BYTE_ORDER == __LITTLE_ENDIAN
-
-struct avctp_header {
- uint8_t ipid:1;
- uint8_t cr:1;
- uint8_t packet_type:2;
- uint8_t transaction:4;
- uint16_t pid;
-} __attribute__ ((packed));
-#define AVCTP_HEADER_LENGTH 3
-
-struct avc_header {
- uint8_t code:4;
- uint8_t _hdr0:4;
- uint8_t subunit_id:3;
- uint8_t subunit_type:5;
- uint8_t opcode;
-} __attribute__ ((packed));
-
-#elif __BYTE_ORDER == __BIG_ENDIAN
-
-struct avctp_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t cr:1;
- uint8_t ipid:1;
- uint16_t pid;
-} __attribute__ ((packed));
-#define AVCTP_HEADER_LENGTH 3
-
-struct avc_header {
- uint8_t _hdr0:4;
- uint8_t code:4;
- uint8_t subunit_type:5;
- uint8_t subunit_id:3;
- uint8_t opcode;
-} __attribute__ ((packed));
-
-#else
-#error "Unknown byte order"
-#endif
-
struct avctp_control_req {
struct avctp_pending_req *p;
uint8_t code;
diff --git a/android/avctp.h b/android/avctp.h
index a22bf13..428eeb8 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
@@ -80,6 +79,50 @@
#define AVC_F3 0x73
#define AVC_F4 0x74

+#if __BYTE_ORDER == __LITTLE_ENDIAN
+
+struct avctp_header {
+ uint8_t ipid:1;
+ uint8_t cr:1;
+ uint8_t packet_type:2;
+ uint8_t transaction:4;
+ uint16_t pid;
+} __attribute__ ((packed));
+#define AVCTP_HEADER_LENGTH 3
+
+struct avc_header {
+ uint8_t code:4;
+ uint8_t _hdr0:4;
+ uint8_t subunit_id:3;
+ uint8_t subunit_type:5;
+ uint8_t opcode;
+} __attribute__ ((packed));
+#define AVC_HEADER_LENGTH 3
+
+#elif __BYTE_ORDER == __BIG_ENDIAN
+
+struct avctp_header {
+ uint8_t transaction:4;
+ uint8_t packet_type:2;
+ uint8_t cr:1;
+ uint8_t ipid:1;
+ uint16_t pid;
+} __attribute__ ((packed));
+#define AVCTP_HEADER_LENGTH 3
+
+struct avc_header {
+ uint8_t _hdr0:4;
+ uint8_t code:4;
+ uint8_t subunit_type:5;
+ uint8_t subunit_id:3;
+ uint8_t opcode;
+} __attribute__ ((packed));
+#define AVC_HEADER_LENGTH 3
+
+#else
+#error "Unknown byte order"
+#endif
+
struct avctp;

typedef bool (*avctp_passthrough_cb) (struct avctp *session,
--
1.8.3.2

2014-01-29 07:56:42

by Andrei Emeltchenko

Subject: Re: [RFC] android/avctp: Move struct definitions to header

Hi Luiz,

On Tue, Jan 28, 2014 at 06:56:25AM -0800, Luiz Augusto von Dentz wrote:
> Hi Andrei,
>
> On Tue, Jan 28, 2014 at 4:34 AM, Andrei Emeltchenko
> <[email protected]> wrote:
> > 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. The patch moves structure definitions to the header in
> > consistent way.
>
> I wonder why you did not do the opposite, move AVC_HEADER_LENGTH into
> avctp.c since the AVC packet control is all done inside avctp.c it
> should probably not be exposed.

I do actually think that avctp.c is wrong place to deal with AVC since
AVCTP shouldn't handle it but anyway I will move it there.

Best regards
Andrei Emeltchenko

2014-01-28 14:56:25

by Luiz Augusto von Dentz

Subject: Re: [RFC] android/avctp: Move struct definitions to header

Hi Andrei,

On Tue, Jan 28, 2014 at 4:34 AM, Andrei Emeltchenko
<[email protected]> wrote:
> 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. The patch moves structure definitions to the header in
> consistent way.

I wonder why you did not do the opposite, move AVC_HEADER_LENGTH into
avctp.c since the AVC packet control is all done inside avctp.c it
should probably not be exposed.