From 214b0eae8aa011fa8b3e8a3dcc784f8d423aeffb Mon Sep 17 00:00:00 2001 From: Petteri Aimonen Date: Tue, 2 Apr 2013 19:55:21 +0300 Subject: [PATCH] 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 --- docs/concepts.rst | 12 +++++++----- docs/reference.rst | 10 +++++++--- example/client.c | 2 +- example/server.c | 4 ++-- pb.h | 9 +++++++++ pb_decode.c | 10 ++++++++-- pb_encode.c | 9 ++++++++- tests/decode_unittests.c | 4 ++-- tests/encode_unittests.c | 4 ++-- tests/test_decode_callbacks.c | 16 ++++++++-------- tests/test_encode_callbacks.c | 8 ++++---- 11 files changed, 58 insertions(+), 30 deletions(-) diff --git a/docs/concepts.rst b/docs/concepts.rst index 052edcc1..4bc0dee8 100644 --- 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 93aae8b0..0db8e43e 100644 --- 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 edc83948..e6e9a2e0 100644 --- 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 346e9fb9..9a9c2644 100644 --- 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 b9c3f759..61649e93 100644 --- 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 6e81b40b..e727f334 100644 --- 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 7acee360..48a3c950 100644 --- 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 039c9fa4..1e74c34d 100644 --- 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 9cdbc66e..3078998e 100644 --- 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 95824d1a..7ce4ec0b 100644 --- a/tests/test_decode_callbacks.c +++ b/tests/test_decode_callbacks.c @@ -6,7 +6,7 @@ #include #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 7fa017f9..afab48e8 100644 --- a/tests/test_encode_callbacks.c +++ b/tests/test_encode_callbacks.c @@ -5,7 +5,7 @@ #include #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; -- 2.16.6