Serialize struct am7xxx_header properly before sending it on the wire

That is in order to:

  1. keep data in the structs always in the host native byte order, this
     is more natural and less error prone as we might forget to use
     htole32() when setting struct fields or le32toh() when accessing
     them;

  2. be more portable: this way the buffer sent to the wire is
     independent of struct alignments or paddings introduced by
     compilers or required by a particular architecture.

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 39f4e86..1228201 100644 (file)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -5,7 +5,7 @@ add_definitions("-D_POSIX_C_SOURCE=2") # for getopt()
 find_package(libusb-1.0 REQUIRED)
 include_directories(${LIBUSB_1_INCLUDE_DIRS})
 
-set(SRC am7xxx.c)
+set(SRC am7xxx.c serialize.c)
 
 # Build the library
 add_library(am7xxx SHARED ${SRC})

diff --git a/src/am7xxx.c b/src/am7xxx.c
index ecef379..182ea27 100644 (file)
--- a/src/am7xxx.c
+++ b/src/am7xxx.c
@@ -17,10 +17,11 @@
  */
 
 #include <stdio.h>
-#include <endian.h>
+#include <stdlib.h>
 #include <errno.h>
 
 #include "am7xxx.h"
+#include "serialize.h"
 
 #define AM7XXX_VENDOR_ID  0x1de1
 #define AM7XXX_PRODUCT_ID 0xc101
@@ -110,16 +111,39 @@ static int send_data(am7xxx_device dev, uint8_t *buffer, unsigned int len)
        return 0;
 }
 
+static void serialize_header(struct am7xxx_header *h, uint8_t *buffer)
+{
+       uint8_t **buffer_iterator = &buffer;
+
+       put_le32(h->packet_type, buffer_iterator);
+       put_8(h->unknown0, buffer_iterator);
+       put_8(h->header_data_len, buffer_iterator);
+       put_8(h->unknown2, buffer_iterator);
+       put_8(h->unknown3, buffer_iterator);
+       put_le32(h->header_data.data.field0, buffer_iterator);
+       put_le32(h->header_data.data.field1, buffer_iterator);
+       put_le32(h->header_data.data.field2, buffer_iterator);
+       put_le32(h->header_data.data.field3, buffer_iterator);
+}
+
 static int send_header(am7xxx_device dev, struct am7xxx_header *h)
 {
-       union {
-               struct am7xxx_header header;
-               uint8_t buffer[sizeof (struct am7xxx_header)];
-       } data;
+       uint8_t *buffer;
+       int ret;
 
-       data.header = *h;
+       buffer = calloc(AM7XXX_HEADER_WIRE_SIZE, 1);
+       if (buffer == NULL) {
+               perror("calloc buffer");
+               return -ENOMEM;
+       }
+
+       serialize_header(h, buffer);
+       ret = send_data(dev, buffer, AM7XXX_HEADER_WIRE_SIZE);
+       if (ret < 0)
+               fprintf(stderr, "send_header: failed to send data.\n");
 
-       return send_data(dev, data.buffer, sizeof (struct am7xxx_header));
+       free(buffer);
+       return ret;
 }
 
 am7xxx_device am7xxx_init(void)
@@ -165,17 +189,17 @@ int am7xxx_send_image(am7xxx_device dev,
 {
        int ret;
        struct am7xxx_header h = {
-               .packet_type     = htole32(AM7XXX_PACKET_TYPE_IMAGE),
+               .packet_type     = AM7XXX_PACKET_TYPE_IMAGE,
                .unknown0        = 0x00,
                .header_data_len = sizeof(struct am7xxx_image_header),
                .unknown2        = 0x3e,
                .unknown3        = 0x10,
                .header_data = {
                        .image = {
-                               .format     = htole32(format),
-                               .width      = htole32(width),
-                               .height     = htole32(height),
-                               .image_size = htole32(size),
+                               .format     = format,
+                               .width      = width,
+                               .height     = height,
+                               .image_size = size,
                        },
                },
        };

diff --git a/src/am7xxx.h b/src/am7xxx.h
index 5b5f111..49510eb 100644 (file)
--- a/src/am7xxx.h
+++ b/src/am7xxx.h
@@ -47,6 +47,13 @@ typedef enum {
        AM7XXX_POWER_HIGH = 3,
 } am7xxx_power_mode;
 
+struct am7xxx_generic_header {
+       uint32_t field0;
+       uint32_t field1;
+       uint32_t field2;
+       uint32_t field3;
+};
+
 struct am7xxx_image_header {
        uint32_t format;
        uint32_t width;
@@ -70,6 +77,12 @@ struct am7xxx_power_header {
  * 04 00 00 00 00 0c ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  */
 
+/* The header size on the wire is known to be always 24 bytes, regardless of
+ * the memory configuration enforced by different architechtures or compilers
+ * for struct am7xxx_header
+ */
+#define AM7XXX_HEADER_WIRE_SIZE 24
+
 struct am7xxx_header {
        uint32_t packet_type;
        uint8_t unknown0;
@@ -77,6 +90,7 @@ struct am7xxx_header {
        uint8_t unknown2;
        uint8_t unknown3;
        union {
+               struct am7xxx_generic_header data;
                struct am7xxx_image_header image;
                struct am7xxx_power_header power;
        } header_data;

diff --git a/src/serialize.c b/src/serialize.c
new file mode 100644 (file)
index 0000000..be300ea
--- /dev/null
+++ b/src/serialize.c
@@ -0,0 +1,61 @@
+/* am7xxx - communication with AM7xxx based USB Pico Projectors and DPFs
+ *
+ * Copyright (C) 2012  Antonio Ospite <ospite@studenti.unina.it>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <string.h>
+#include <endian.h>
+
+#include "serialize.h"
+
+uint8_t get_8(uint8_t **bufferp)
+{
+       uint8_t tmp;
+
+       tmp = *bufferp[0];
+       *bufferp += 1;
+
+       return tmp;
+}
+
+uint32_t get_le32(uint8_t **bufferp)
+{
+       uint32_t tmp;
+
+       memcpy(&tmp, *bufferp, sizeof (tmp));
+       *bufferp += sizeof (tmp);
+
+       return le32toh(tmp);
+}
+
+uint8_t *put_8(uint8_t value, uint8_t **bufferp)
+{
+       *bufferp[0] = value;
+       *bufferp += 1;
+
+       return *bufferp;
+}
+
+uint8_t *put_le32(uint32_t value, uint8_t **bufferp)
+{
+       uint32_t tmp;
+
+       tmp = htole32(value);
+       memcpy(*bufferp, &tmp, sizeof (tmp));
+       *bufferp += sizeof (tmp);
+
+       return *bufferp;
+}

diff --git a/src/serialize.h b/src/serialize.h
new file mode 100644 (file)
index 0000000..e2dd7d4
--- /dev/null
+++ b/src/serialize.h
@@ -0,0 +1,36 @@
+/* am7xxx - communication with AM7xxx based USB Pico Projectors and DPFs
+ *
+ * Copyright (C) 2012  Antonio Ospite <ospite@studenti.unina.it>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* You can transform a serializazion block of code which uses put-* into the
+ * correspondent unserialization block with this vim substitution pattern:
+ *
+ *   s/put_\([^(]*\)(\([^,]*\),\s*\([^)]*\))/\2 = get_\1(\3)/g
+ */
+
+#ifndef __SERIALIZE_H
+#define __SERIALIZE_H
+
+#include <stdint.h>
+
+uint8_t get_8(uint8_t **bufferp);
+uint32_t get_le32(uint8_t **bufferp);
+
+uint8_t *put_8(uint8_t value, uint8_t **bufferp);
+uint8_t *put_le32(uint32_t value, uint8_t **bufferp);
+
+#endif /* __SERIALIZE_H */