Change the callback function to use void**.

NOTE: This change breaks backwards-compatibility by default.
If you have old callback functions, you can define PB_OLD_CALLBACK_STYLE
to retain the old behaviour.

If you want to convert your old callbacks to new signature, you need
to do the following:

1) Change decode callback argument to   void **arg
      and encode callback argument to   void * const *arg.

2) Change any reference to arg into *arg.

The rationale for making the new behaviour the default is that it
simplifies the common case of "allocate some memory in decode callback".

Update issue 69
Status: FixedInGit

diff --git a/docs/concepts.rst b/docs/concepts.rst
index 052edcc..4bc0dee 100644 (file)
--- a/docs/concepts.rst
+++ b/docs/concepts.rst
@@ -181,7 +181,9 @@ Field callbacks
 ===============
 When a field has dynamic length, nanopb cannot statically allocate storage for it. Instead, it allows you to handle the field in whatever way you want, using a callback function.
 
-The `pb_callback_t`_ structure contains a function pointer and a *void* pointer you can use for passing data to the callback. If the function pointer is NULL, the field will be skipped. The actual behavior of the callback function is different in encoding and decoding modes.
+The `pb_callback_t`_ structure contains a function pointer and a *void* pointer called *arg* you can use for passing data to the callback. If the function pointer is NULL, the field will be skipped. A pointer to the *arg* is passed to the function, so that it can modify it and retrieve the value.
+
+The actual behavior of the callback function is different in encoding and decoding modes. In encoding mode, the callback is called once and should write out everything, including field tags. In decoding mode, the callback is called repeatedly for every data item.
 
 .. _`pb_callback_t`: reference.html#pb-callback-t
 
@@ -189,7 +191,7 @@ Encoding callbacks
 ------------------
 ::
 
-    bool (*encode)(pb_ostream_t *stream, const pb_field_t *field, const void *arg);
+    bool (*encode)(pb_ostream_t *stream, const pb_field_t *field, void * const *arg);
 
 When encoding, the callback should write out complete fields, including the wire type and field number tag. It can write as many or as few fields as it likes. For example, if you want to write out an array as *repeated* field, you should do it all in a single call.
 
@@ -203,7 +205,7 @@ If the callback is used in a submessage, it will be called multiple times during
 
 This callback writes out a dynamically sized string::
 
-    bool write_string(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+    bool write_string(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
     {
         char *str = get_string_from_somewhere();
         if (!pb_encode_tag_for_field(stream, field))
@@ -216,7 +218,7 @@ Decoding callbacks
 ------------------
 ::
 
-    bool (*decode)(pb_istream_t *stream, const pb_field_t *field, void *arg);
+    bool (*decode)(pb_istream_t *stream, const pb_field_t *field, void **arg);
 
 When decoding, the callback receives a length-limited substring that reads the contents of a single field. The field tag has already been read. For *string* and *bytes*, the length value has already been parsed, and is available at *stream->bytes_left*.
 
@@ -226,7 +228,7 @@ The callback will be called multiple times for repeated fields. For packed field
 
 This callback reads multiple integers and prints them::
 
-    bool read_ints(pb_istream_t *stream, const pb_field_t *field, void *arg)
+    bool read_ints(pb_istream_t *stream, const pb_field_t *field, void **arg)
     {
         while (stream->bytes_left)
         {

diff --git a/docs/reference.rst b/docs/reference.rst
index 93aae8b..0db8e43 100644 (file)
--- a/docs/reference.rst
+++ b/docs/reference.rst
@@ -24,6 +24,8 @@ PB_NO_ERRMSG                   Disables the support for error messages; only err
                                Decreases the code size by a few hundred bytes.
 PB_BUFFER_ONLY                 Disables the support for custom streams. Only supports encoding to memory buffers.
                                Speeds up execution and decreases code size slightly.
+PB_OLD_CALLBACK_STYLE          Use the old function signature (void\* instead of void\*\*) for callback fields. This was the
+                               default until nanopb-0.2.1.
 ============================  ================================================================================================
 
 The PB_MAX_REQUIRED_FIELDS, PB_FIELD_16BIT and PB_FIELD_32BIT settings allow raising some datatype limits to suit larger messages.
@@ -122,14 +124,16 @@ Part of a message structure, for fields with type PB_HTYPE_CALLBACK::
     typedef struct _pb_callback_t pb_callback_t;
     struct _pb_callback_t {
         union {
-            bool (*decode)(pb_istream_t *stream, const pb_field_t *field, void *arg);
-            bool (*encode)(pb_ostream_t *stream, const pb_field_t *field, const void *arg);
+            bool (*decode)(pb_istream_t *stream, const pb_field_t *field, void **arg);
+            bool (*encode)(pb_ostream_t *stream, const pb_field_t *field, void * const *arg);
         } funcs;
         
         void *arg;
     };
 
-The *arg* is passed to the callback when calling. It can be used to store any information that the callback might need.
+A pointer to the *arg* is passed to the callback when calling. It can be used to store any information that the callback might need.
+
+Previously the function received just the value of *arg* instead of a pointer to it. This old behaviour can be enabled by defining *PB_OLD_CALLBACK_STYLE*.
 
 When calling `pb_encode`_, *funcs.encode* is used, and similarly when calling `pb_decode`_, *funcs.decode* is used. The function pointers are stored in the same memory location but are of incompatible types. You can set the function pointer to NULL to skip the field.
 

diff --git a/example/client.c b/example/client.c
index edc8394..e6e9a2e 100644 (file)
--- a/example/client.c
+++ b/example/client.c
@@ -23,7 +23,7 @@
 #include "fileproto.pb.h"
 #include "common.h"
 
-bool printfile_callback(pb_istream_t *stream, const pb_field_t *field, void *arg)
+bool printfile_callback(pb_istream_t *stream, const pb_field_t *field, void **arg)
 {
     FileInfo fileinfo;
     

diff --git a/example/server.c b/example/server.c
index 346e9fb..9a9c264 100644 (file)
--- a/example/server.c
+++ b/example/server.c
@@ -23,9 +23,9 @@
 #include "fileproto.pb.h"
 #include "common.h"
 
-bool listdir_callback(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+bool listdir_callback(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
 {
-    DIR *dir = (DIR*) arg;
+    DIR *dir = (DIR*) *arg;
     struct dirent *file;
     FileInfo fileinfo;
     

diff --git a/pb.h b/pb.h
index b9c3f75..61649e9 100644 (file)
--- a/pb.h
+++ b/pb.h
@@ -203,10 +203,19 @@ typedef struct _pb_istream_t pb_istream_t;
 typedef struct _pb_ostream_t pb_ostream_t;
 typedef struct _pb_callback_t pb_callback_t;
 struct _pb_callback_t {
+#ifdef PB_OLD_CALLBACK_STYLE
+    /* Deprecated since nanopb-0.2.1 */
     union {
         bool (*decode)(pb_istream_t *stream, const pb_field_t *field, void *arg);
         bool (*encode)(pb_ostream_t *stream, const pb_field_t *field, const void *arg);
     } funcs;
+#else
+    /* New function signature, which allows modifying arg contents in callback. */
+    union {
+        bool (*decode)(pb_istream_t *stream, const pb_field_t *field, void **arg);
+        bool (*encode)(pb_ostream_t *stream, const pb_field_t *field, void * const *arg);
+    } funcs;
+#endif    
     
     /* Free arg for use by callback */
     void *arg;

diff --git a/pb_decode.c b/pb_decode.c
index 6e81b40..e727f33 100644 (file)
--- a/pb_decode.c
+++ b/pb_decode.c
@@ -414,6 +414,12 @@ static bool checkreturn decode_callback_field(pb_istream_t *stream, pb_wire_type
 {
     pb_callback_t *pCallback = (pb_callback_t*)iter->pData;
     
+#ifdef PB_OLD_CALLBACK_STYLE
+    void *arg = pCallback->arg;
+#else
+    void **arg = &(pCallback->arg);
+#endif
+    
     if (pCallback->funcs.decode == NULL)
         return pb_skip_field(stream, wire_type);
     
@@ -426,7 +432,7 @@ static bool checkreturn decode_callback_field(pb_istream_t *stream, pb_wire_type
         
         while (substream.bytes_left)
         {
-            if (!pCallback->funcs.decode(&substream, iter->pos, pCallback->arg))
+            if (!pCallback->funcs.decode(&substream, iter->pos, arg))
                 PB_RETURN_ERROR(stream, "callback failed");
         }
         
@@ -447,7 +453,7 @@ static bool checkreturn decode_callback_field(pb_istream_t *stream, pb_wire_type
             return false;
         substream = pb_istream_from_buffer(buffer, size);
         
-        return pCallback->funcs.decode(&substream, iter->pos, pCallback->arg);
+        return pCallback->funcs.decode(&substream, iter->pos, arg);
     }
 }
 

diff --git a/pb_encode.c b/pb_encode.c
index 7acee36..48a3c95 100644 (file)
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -198,9 +198,16 @@ bool checkreturn encode_static_field(pb_ostream_t *stream, const pb_field_t *fie
 bool checkreturn encode_callback_field(pb_ostream_t *stream, const pb_field_t *field, const void *pData)
 {
     const pb_callback_t *callback = (const pb_callback_t*)pData;
+    
+#ifdef PB_OLD_CALLBACK_STYLE
+    const void *arg = callback->arg;
+#else
+    void * const *arg = &(callback->arg);
+#endif    
+    
     if (callback->funcs.encode != NULL)
     {
-        if (!callback->funcs.encode(stream, field, callback->arg))
+        if (!callback->funcs.encode(stream, field, arg))
             PB_RETURN_ERROR(stream, "callback error");
     }
     return true;

diff --git a/tests/decode_unittests.c b/tests/decode_unittests.c
index 039c9fa..1e74c34 100644 (file)
--- a/tests/decode_unittests.c
+++ b/tests/decode_unittests.c
@@ -19,11 +19,11 @@ bool stream_callback(pb_istream_t *stream, uint8_t *buf, size_t count)
 }
 
 /* Verifies that the stream passed to callback matches the byte array pointed to by arg. */
-bool callback_check(pb_istream_t *stream, const pb_field_t *field, void *arg)
+bool callback_check(pb_istream_t *stream, const pb_field_t *field, void **arg)
 {
     int i;
     uint8_t byte;
-    pb_bytes_array_t *ref = (pb_bytes_array_t*) arg;
+    pb_bytes_array_t *ref = (pb_bytes_array_t*) *arg;
     
     for (i = 0; i < ref->size; i++)
     {

diff --git a/tests/encode_unittests.c b/tests/encode_unittests.c
index 9cdbc66..3078998 100644 (file)
--- a/tests/encode_unittests.c
+++ b/tests/encode_unittests.c
@@ -17,7 +17,7 @@ bool streamcallback(pb_ostream_t *stream, const uint8_t *buf, size_t count)
     return true;
 }
 
-bool fieldcallback(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+bool fieldcallback(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
 {
     int value = 0x55;
     if (!pb_encode_tag_for_field(stream, field))
@@ -25,7 +25,7 @@ bool fieldcallback(pb_ostream_t *stream, const pb_field_t *field, const void *ar
     return pb_encode_varint(stream, value);
 }
 
-bool crazyfieldcallback(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+bool crazyfieldcallback(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
 {
     /* This callback writes different amount of data the second time. */
     uint32_t *state = (uint32_t*)arg;

diff --git a/tests/test_decode_callbacks.c b/tests/test_decode_callbacks.c
index 95824d1..7ce4ec0 100644 (file)
--- a/tests/test_decode_callbacks.c
+++ b/tests/test_decode_callbacks.c
@@ -6,7 +6,7 @@
 #include <pb_decode.h>
 #include "callbacks.pb.h"
 
-bool print_string(pb_istream_t *stream, const pb_field_t *field, void *arg)
+bool print_string(pb_istream_t *stream, const pb_field_t *field, void **arg)
 {
     uint8_t buffer[1024] = {0};
     
@@ -20,37 +20,37 @@ bool print_string(pb_istream_t *stream, const pb_field_t *field, void *arg)
     /* Print the string, in format comparable with protoc --decode.
      * Format comes from the arg defined in main().
      */
-    printf((char*)arg, buffer);
+    printf((char*)*arg, buffer);
     return true;
 }
 
-bool print_int32(pb_istream_t *stream, const pb_field_t *field, void *arg)
+bool print_int32(pb_istream_t *stream, const pb_field_t *field, void **arg)
 {
     uint64_t value;
     if (!pb_decode_varint(stream, &value))
         return false;
     
-    printf((char*)arg, (long)value);
+    printf((char*)*arg, (long)value);
     return true;
 }
 
-bool print_fixed32(pb_istream_t *stream, const pb_field_t *field, void *arg)
+bool print_fixed32(pb_istream_t *stream, const pb_field_t *field, void **arg)
 {
     uint32_t value;
     if (!pb_decode_fixed32(stream, &value))
         return false;
     
-    printf((char*)arg, (long)value);
+    printf((char*)*arg, (long)value);
     return true;
 }
 
-bool print_fixed64(pb_istream_t *stream, const pb_field_t *field, void *arg)
+bool print_fixed64(pb_istream_t *stream, const pb_field_t *field, void **arg)
 {
     uint64_t value;
     if (!pb_decode_fixed64(stream, &value))
         return false;
     
-    printf((char*)arg, (long long)value);
+    printf((char*)*arg, (long long)value);
     return true;
 }
 

diff --git a/tests/test_encode_callbacks.c b/tests/test_encode_callbacks.c
index 7fa017f..afab48e 100644 (file)
--- a/tests/test_encode_callbacks.c
+++ b/tests/test_encode_callbacks.c
@@ -5,7 +5,7 @@
 #include <pb_encode.h>
 #include "callbacks.pb.h"
 
-bool encode_string(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+bool encode_string(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
 {
     char *str = "Hello world!";
     
@@ -15,7 +15,7 @@ bool encode_string(pb_ostream_t *stream, const pb_field_t *field, const void *ar
     return pb_encode_string(stream, (uint8_t*)str, strlen(str));
 }
 
-bool encode_int32(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+bool encode_int32(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
 {
     if (!pb_encode_tag_for_field(stream, field))
         return false;
@@ -23,7 +23,7 @@ bool encode_int32(pb_ostream_t *stream, const pb_field_t *field, const void *arg
     return pb_encode_varint(stream, 42);
 }
 
-bool encode_fixed32(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+bool encode_fixed32(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
 {
     if (!pb_encode_tag_for_field(stream, field))
         return false;
@@ -32,7 +32,7 @@ bool encode_fixed32(pb_ostream_t *stream, const pb_field_t *field, const void *a
     return pb_encode_fixed32(stream, &value);
 }
 
-bool encode_fixed64(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+bool encode_fixed64(pb_ostream_t *stream, const pb_field_t *field, void * const *arg)
 {
     if (!pb_encode_tag_for_field(stream, field))
         return false;