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

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

[Andrei Emeltchenko]

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

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

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

[Luiz Augusto von Dentz]

Hi Andrei,

On Tue, Jan 28, 2014 at 4:34 AM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> 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.

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

[Andrei Emeltchenko]

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
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > 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