marshal: fix list marshalling and various other changes

Since last element and previous element are a pointer to a pointer, we
should handle this case differently. Unit tests are not failing
because the original memory space is still available on the stack. We
just ignore last and previous. This still allows to use the list for
the most useful operations: TAILQ_EMPTY, TAILQ_FOREACH and
TAILQ_REMOVE. We can't use TAILQ_INSERT_TAIL!

Another important change is that by default, macros will declare an
external structure. MARSHAL_EXPORT variable should be defined only for
one module. Otherwise, marshal structures will be defined several
times.

We also rename `_marshal_*` to `marshal_*_`. This enables more
graceful logging messages.

diff --git a/src/marshal.c b/src/marshal.c
index 303069a66789eca9192a8fd787e37a98ea6572e8..44c527d4af05ffbc0e33f2f8b9e5187a9b4397c0 100644 (file)
--- a/src/marshal.c
+++ b/src/marshal.c
@@ -14,6 +14,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#define MARSHAL_EXPORT
 #include "lldpd.h"
 
 /* A serialized object */
@@ -23,17 +24,17 @@ struct marshal_serialized {
        unsigned char object[0];
 };
 
-struct marshal_info marshal_info__string = {
+struct marshal_info marshal_info_string = {
        .name = "null string",
        .size = 0,
        .pointers = {{ .mi = NULL }},
 };
-struct marshal_info marshal_info__fstring = {
+struct marshal_info marshal_info_fstring = {
        .name = "fixed string",
        .size = 0,
        .pointers = {{ .mi = NULL }},
 };
-struct marshal_info marshal_info__ignore = {
+struct marshal_info marshal_info_ignore = {
        .name = "ignored",
        .size = 0,
        .pointers = {{ .mi = NULL }},
@@ -48,7 +49,7 @@ TAILQ_HEAD(ref_l, ref);
 
 /* Serialize the given object. */
 size_t
-_marshal_serialize(struct marshal_info *mi, void *unserialized, void **input,
+marshal_serialize_(struct marshal_info *mi, void *unserialized, void **input,
     int skip, void *_refs, int osize)
 {
        /* Check if we have already serialized this one. */
@@ -111,8 +112,9 @@ _marshal_serialize(struct marshal_info *mi, void *unserialized, void **input,
                        if (source == NULL) continue;
                } else
                        source = (void *)((unsigned char *)unserialized + current->offset);
-               memcpy(&osize, (unsigned char*)unserialized + current->offset2, sizeof(int));
-               sublen = _marshal_serialize(current->mi,
+               if (current->offset2)
+                       memcpy(&osize, (unsigned char*)unserialized + current->offset2, sizeof(int));
+               sublen = marshal_serialize_(current->mi,
                    source, &target,
                    current->kind == substruct, refs, osize);
                if (sublen == -1) {
@@ -197,7 +199,7 @@ marshal_free(struct gc_l *pointers, int gconly)
 
 /* Unserialize the given object. */
 size_t
-_marshal_unserialize(struct marshal_info *mi, void *buffer, size_t len, void **output,
+marshal_unserialize_(struct marshal_info *mi, void *buffer, size_t len, void **output,
     void *_pointers, int skip, int osize)
 {
        int    total_len = sizeof(struct marshal_serialized) + (skip?0:mi->size);
@@ -271,8 +273,9 @@ _marshal_unserialize(struct marshal_info *mi, void *buffer, size_t len, void **o
                        if (already) continue;
                }
                /* Deserialize */
-               memcpy(&osize, (unsigned char *)*output + current->offset2, sizeof(int));
-               sublen = _marshal_unserialize(current->mi,
+               if (current->offset2)
+                       memcpy(&osize, (unsigned char *)*output + current->offset2, sizeof(int));
+               sublen = marshal_unserialize_(current->mi,
                    (unsigned char *)buffer + total_len, len - total_len, &new, pointers,
                    current->kind == substruct, osize);
                if (sublen == 0) {

diff --git a/src/marshal.h b/src/marshal.h
index 74640306e54f9b9c0dd7c896b0615c805a46b926..91b99d5d5d094d29e24c531f3c380237bc17555a 100644 (file)
--- a/src/marshal.h
+++ b/src/marshal.h
@@ -37,13 +37,15 @@ struct marshal_info {
        struct marshal_subinfo pointers[]; /* Pointer to other structures */
 };
 /* Special case for strings */
-extern struct marshal_info marshal_info__string;
-extern struct marshal_info marshal_info__fstring;
-extern struct marshal_info marshal_info__ignore;
+extern struct marshal_info marshal_info_string;
+extern struct marshal_info marshal_info_fstring;
+extern struct marshal_info marshal_info_ignore;
 
 /* Declare a new marshal_info struct named after the type we want to
    marshal. The marshalled type has to be a structure. */
-#define MARSHAL_BEGIN(type) struct marshal_info marshal_info_##type = \
+#define MARSHAL_INFO(type) marshal_info_##type
+#ifdef MARSHAL_EXPORT
+#define MARSHAL_BEGIN(type) struct marshal_info MARSHAL_INFO(type) =   \
        {                                                               \
                .name = #type,                                          \
                .size = sizeof(struct type),                            \
@@ -51,27 +53,37 @@ extern struct marshal_info marshal_info__ignore;
 #define MARSHAL_ADD(_kind, type, subtype, member)              \
        { .offset = offsetof(struct type, member),              \
          .kind = _kind,                                        \
-         .mi = &marshal_info_##subtype },
-#define MARSHAL_POINTER(...) MARSHAL_ADD(pointer, ##__VA_ARGS__)
-#define MARSHAL_SUBSTRUCT(...) MARSHAL_ADD(substruct, ##__VA_ARGS__)
-#define MARSHAL_STR(type, member) MARSHAL_ADD(pointer, type, _string, member)
+         .mi = &MARSHAL_INFO(subtype) },
 #define MARSHAL_FSTR(type, member, len)                                \
        { .offset = offsetof(struct type, member),              \
          .offset2 = offsetof(struct type, len),                \
          .kind = pointer,                                      \
-         .mi = &marshal_info__fstring },
-#define MARSHAL_IGNORE(type, member) MARSHAL_ADD(ignore, type, _ignore, member)
+         .mi = &marshal_info_fstring },
+#define MARSHAL_END { .mi = NULL } } }
+#else
+#define MARSHAL_BEGIN(type) extern struct marshal_info MARSHAL_INFO(type)
+#define MARSHAL_ADD(...)
+#define MARSHAL_FSTR(...)
+#define MARSHAL_END
+#endif
+/* Shortcuts */
+#define MARSHAL_POINTER(...) MARSHAL_ADD(pointer, ##__VA_ARGS__)
+#define MARSHAL_SUBSTRUCT(...) MARSHAL_ADD(substruct, ##__VA_ARGS__)
+#define MARSHAL_STR(type, member) MARSHAL_ADD(pointer, type, string, member)
+#define MARSHAL_IGNORE(type, member) MARSHAL_ADD(ignore, type, ignore, member)
 #define MARSHAL_TQE(type, field)                        \
        MARSHAL_POINTER(type, type, field.tqe_next)      \
-       MARSHAL_POINTER(type, type, field.tqe_prev)
+       MARSHAL_IGNORE(type, field.tqe_prev)
+/* Support for TAILQ list is partial. Access to last and previous
+   elements is not available. Some operations are therefore not
+   possible. However, TAILQ_FOREACH and TAILQ_REMOVE are still
+   available. */
 #define MARSHAL_TQH(type, subtype)                      \
        MARSHAL_POINTER(type, subtype, tqh_first)        \
-       MARSHAL_POINTER(type, subtype, tqh_last)
+       MARSHAL_IGNORE(type, tqh_last)
 #define MARSHAL_SUBTQ(type, subtype, field)             \
        MARSHAL_POINTER(type, subtype, field.tqh_first)  \
-       MARSHAL_POINTER(type, subtype, field.tqh_last)
-#define MARSHAL_END { .mi = NULL } } }
-/* Shortcuts */
+       MARSHAL_IGNORE(type, field.tqh_last)
 #define MARSHAL(type)                  \
        MARSHAL_BEGIN(type)             \
        MARSHAL_END
@@ -81,12 +93,12 @@ extern struct marshal_info marshal_info__ignore;
        MARSHAL_END
 
 /* Serialization */
-size_t  _marshal_serialize(struct marshal_info *, void *, void **, int, void *, int);
-#define marshal_serialize(type, o, output) _marshal_serialize(&marshal_info_##type, o, output, 0, NULL, 0)
+size_t  marshal_serialize_(struct marshal_info *, void *, void **, int, void *, int);
+#define marshal_serialize(type, o, output) marshal_serialize_(&MARSHAL_INFO(type), o, output, 0, NULL, 0)
 
 /* Unserialization */
-size_t  _marshal_unserialize(struct marshal_info *, void *, size_t, void **, void*, int, int);
+size_t  marshal_unserialize_(struct marshal_info *, void *, size_t, void **, void*, int, int);
 #define marshal_unserialize(type, o, l, input) \
-       _marshal_unserialize(&marshal_info_##type, o, l, (void **)input, NULL, 0, 0)
+       marshal_unserialize_(&MARSHAL_INFO(type), o, l, (void **)input, NULL, 0, 0)
 
 #endif

diff --git a/tests/check_marshal.c b/tests/check_marshal.c
index 0ed35ce8d4a3e42a75bef87d2df8bb073afb3b61..a2bf6dc1a44d2646d8ef9c1bcacaa4513554fa12 100644 (file)
--- a/tests/check_marshal.c
+++ b/tests/check_marshal.c
@@ -1,11 +1,16 @@
+#include <stdlib.h>
+#include <unistd.h>
 #include <check.h>
+#include <sys/queue.h>
 
-#include "../src/lldpd.h"
+#define MARSHAL_EXPORT
+#include "../src/marshal.h"
 
 /* This suite can be run in valgrind for memory leaks:
    CK_FORK=no valgrind -v --leak-check=yes ./tests/check_marshal
 */
 
+
 struct struct_simple {
        int a1;
        long a2;