Fix oneof submessage initialization bug.

Update issue 149
Status: FixedInGit

examples/cmake_simple/CMakeLists.txt

@@ -0,0 +1,17 @@
+cmake_minimum_required(VERSION 2.8)
+project(NANOPB_CMAKE_SIMPLE C)
+
+set(NANOPB_SRC_ROOT_FOLDER ${CMAKE_CURRENT_SOURCE_DIR}/../..)
+set(CMAKE_MODULE_PATH ${NANOPB_SRC_ROOT_FOLDER}/extra)
+find_package(Nanopb REQUIRED)
+include_directories(${NANOPB_INCLUDE_DIRS})
+
+nanopb_generate_cpp(PROTO_SRCS PROTO_HDRS simple.proto)
+include_directories(${CMAKE_CURRENT_BINARY_DIR})
+#add_custom_target(generate_proto_sources DEPENDS ${PROTO_SRCS} ${PROTO_HDRS})
+set_source_files_properties(${PROTO_SRCS} ${PROTO_HDRS}
+    PROPERTIES GENERATED TRUE)
+
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Werror -g -O0")
+
+add_executable(simple simple.c ${PROTO_SRCS} ${PROTO_HDRS})

examples/cmake_simple/README.txt

@@ -0,0 +1,18 @@
+Nanopb example "simple" using CMake
+=======================
+
+This example is the same as the simple nanopb example but built using CMake.
+
+Example usage
+-------------
+
+On Linux, create a build directory and then call cmake:
+
+    nanopb/examples/cmake_simple$ mkdir build
+    nanopb/examples/cmake_simple$ cd build/
+    nanopb/examples/cmake_simple/build$ cmake ..
+    nanopb/examples/cmake_simple/build$ make
+
+After that, you can run it with the command: ./simple
+
+On other platforms supported by CMake, refer to CMake instructions.

examples/cmake_simple/simple.c

@@ -0,0 +1,68 @@
+#include <stdio.h>
+#include <pb_encode.h>
+#include <pb_decode.h>
+#include "simple.pb.h"
+
+int main()
+{
+    /* This is the buffer where we will store our message. */
+    uint8_t buffer[128];
+    size_t message_length;
+    bool status;
+    
+    /* Encode our message */
+    {
+        /* Allocate space on the stack to store the message data.
+         *
+         * Nanopb generates simple struct definitions for all the messages.
+         * - check out the contents of simple.pb.h! */
+        SimpleMessage message;
+        
+        /* Create a stream that will write to our buffer. */
+        pb_ostream_t stream = pb_ostream_from_buffer(buffer, sizeof(buffer));
+        
+        /* Fill in the lucky number */
+        message.lucky_number = 13;
+        
+        /* Now we are ready to encode the message! */
+        status = pb_encode(&stream, SimpleMessage_fields, &message);
+        message_length = stream.bytes_written;
+        
+        /* Then just check for any errors.. */
+        if (!status)
+        {
+            printf("Encoding failed: %s\n", PB_GET_ERROR(&stream));
+            return 1;
+        }
+    }
+    
+    /* Now we could transmit the message over network, store it in a file or
+     * wrap it to a pigeon's leg.
+     */
+
+    /* But because we are lazy, we will just decode it immediately. */
+    
+    {
+        /* Allocate space for the decoded message. */
+        SimpleMessage message;
+        
+        /* Create a stream that reads from the buffer. */
+        pb_istream_t stream = pb_istream_from_buffer(buffer, message_length);
+        
+        /* Now we are ready to decode the message. */
+        status = pb_decode(&stream, SimpleMessage_fields, &message);
+        
+        /* Check for errors... */
+        if (!status)
+        {
+            printf("Decoding failed: %s\n", PB_GET_ERROR(&stream));
+            return 1;
+        }
+        
+        /* Print the data contained in the message. */
+        printf("Your lucky number was %d!\n", message.lucky_number);
+    }
+    
+    return 0;
+}
+

examples/cmake_simple/simple.proto

@@ -0,0 +1,7 @@
+// A very simple protocol definition, consisting of only
+// one message.
+
+message SimpleMessage {
+    required int32 lucky_number = 1;
+}
+

extra/FindNanopb.cmake

@@ -128,10 +128,36 @@ function(NANOPB_GENERATE_CPP SRCS HDRS)
   set(${SRCS})
   set(${HDRS})
   get_filename_component(GENERATOR_PATH ${NANOPB_GENERATOR_EXECUTABLE} PATH)
+  set(GENERATOR_CORE_DIR ${GENERATOR_PATH}/proto)
+  set(GENERATOR_CORE_SRC
+      ${GENERATOR_CORE_DIR}/nanopb.proto
+      ${GENERATOR_CORE_DIR}/plugin.proto)
+
+  set(GENERATOR_CORE_PYTHON_SRC)
+  foreach(FIL ${GENERATOR_CORE_SRC})
+      get_filename_component(ABS_FIL ${FIL} ABSOLUTE)
+      get_filename_component(FIL_WE ${FIL} NAME_WE)
+
+      set(output "${GENERATOR_CORE_DIR}/${FIL_WE}_pb2.py")
+      set(GENERATOR_CORE_PYTHON_SRC ${GENERATOR_CORE_PYTHON_SRC} ${output})
+      add_custom_command(
+        OUTPUT ${output}
+        COMMAND ${PROTOBUF_PROTOC_EXECUTABLE}
+        ARGS -I${GENERATOR_PATH}/proto
+          --python_out=${GENERATOR_CORE_DIR} ${ABS_FIL}
+        DEPENDS ${ABS_FIL}
+        VERBATIM)
+  endforeach()
 
   foreach(FIL ${ARGN})
     get_filename_component(ABS_FIL ${FIL} ABSOLUTE)
     get_filename_component(FIL_WE ${FIL} NAME_WE)
+    get_filename_component(FIL_DIR ${FIL} PATH)
+    set(NANOPB_OPTIONS_FILE ${FIL_DIR}/${FIL_WE}.options)
+    set(NANOPB_OPTIONS)
+    if(EXISTS ${NANOPB_OPTIONS_FILE})
+        set(NANOPB_OPTIONS -f ${NANOPB_OPTIONS_FILE})
+    endif()
 
     list(APPEND ${SRCS} "${CMAKE_CURRENT_BINARY_DIR}/${FIL_WE}.pb.c")
     list(APPEND ${HDRS} "${CMAKE_CURRENT_BINARY_DIR}/${FIL_WE}.pb.h")
@@ -139,16 +165,18 @@ function(NANOPB_GENERATE_CPP SRCS HDRS)
     add_custom_command(
       OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${FIL_WE}.pb"
       COMMAND  ${PROTOBUF_PROTOC_EXECUTABLE}
-      ARGS -I${GENERATOR_PATH} -I${CMAKE_CURRENT_BINARY_DIR} ${_nanobp_include_path} -o${FIL_WE}.pb ${ABS_FIL}
-      DEPENDS ${ABS_FIL}
+      ARGS -I${GENERATOR_PATH} -I${GENERATOR_CORE_DIR}
+        -I${CMAKE_CURRENT_BINARY_DIR} ${_nanobp_include_path}
+        -o${FIL_WE}.pb ${ABS_FIL}
+      DEPENDS ${ABS_FIL} ${GENERATOR_CORE_PYTHON_SRC}
       COMMENT "Running C++ protocol buffer compiler on ${FIL}"
       VERBATIM )
 
     add_custom_command(
       OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${FIL_WE}.pb.c"
              "${CMAKE_CURRENT_BINARY_DIR}/${FIL_WE}.pb.h"
-      COMMAND python
-      ARGS ${NANOPB_GENERATOR_EXECUTABLE} ${FIL_WE}.pb
+      COMMAND ${PYTHON2_EXECUTABLE}
+      ARGS ${NANOPB_GENERATOR_EXECUTABLE} ${FIL_WE}.pb ${NANOPB_OPTIONS}
       DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/${FIL_WE}.pb"
       COMMENT "Running nanopb generator on ${FIL_WE}.pb"
       VERBATIM )
@@ -216,6 +244,18 @@ find_file(NANOPB_GENERATOR_EXECUTABLE
 )
 mark_as_advanced(NANOPB_GENERATOR_EXECUTABLE)
 
+# If python3 has already been found, save it and look for python2.6
+if(${PYTHON_VERSION_MAJOR} AND ${PYTHON_VERSION_MAJOR} EQUAL 3)
+    set(PYTHON3_EXECUTABLE ${PYTHON_EXECUTABLE})
+    set(PYTHON_EXECUTABLE PYTHON_EXECUTABLE-NOTFOUND)
+    find_package(PythonInterp 2.6 REQUIRED)
+    set(PYTHON2_EXECUTABLE ${PYTHON_EXECUTABLE})
+    set(PYTHON_EXECUTABLE ${PYTHON3_EXECUTABLE})
+else()
+    find_package(PythonInterp 2.6 REQUIRED)
+    set(PYTHON2_EXECUTABLE ${PYTHON_EXECUTABLE})
+endif()
+
 include(FindPackageHandleStandardArgs)
 FIND_PACKAGE_HANDLE_STANDARD_ARGS(NANOPB DEFAULT_MSG
   NANOPB_INCLUDE_DIRS

generator/nanopb_generator.py

@@ -1,7 +1,7 @@
 #!/usr/bin/python
 
 '''Generate header file for nanopb from a ProtoBuf FileDescriptorSet.'''
-nanopb_version = "nanopb-0.3.2"
+nanopb_version = "nanopb-0.3.3-dev"
 
 import sys
 
@@ -587,6 +587,9 @@ class OneOf(Field):
         self.name = oneof_desc.name
         self.ctype = 'union'
         self.fields = []
+        self.allocation = 'ONEOF'
+        self.default = None
+        self.rules = 'ONEOF'
 
     def add_field(self, field):
         if field.allocation == 'CALLBACK':
@@ -633,7 +636,6 @@ class OneOf(Field):
         return '\n'.join([f.tags() for f in self.fields])
 
     def pb_field_t(self, prev_field_name):
-        prev_field_name = prev_field_name or self.name
         result = ',\n'.join([f.pb_field_t(prev_field_name) for f in self.fields])
         return result
 
@@ -1124,14 +1126,28 @@ def read_options_file(infile):
         [(namemask, options), ...]
     '''
     results = []
-    for line in infile:
+    for i, line in enumerate(infile):
         line = line.strip()
         if not line or line.startswith('//') or line.startswith('#'):
             continue
         
         parts = line.split(None, 1)
+        
+        if len(parts) < 2:
+            sys.stderr.write("%s:%d: " % (infile.name, i + 1) +
+                             "Option lines should have space between field name and options. " +
+                             "Skipping line: '%s'\n" % line)
+            continue
+        
         opts = nanopb_pb2.NanoPBOptions()
-        text_format.Merge(parts[1], opts)
+        
+        try:
+            text_format.Merge(parts[1], opts)
+        except Exception, e:
+            sys.stderr.write("%s:%d: " % (infile.name, i + 1) +
+                             "Unparseable option line: '%s'. " % line +
+                             "Error: %s\n" % str(e))
+            continue
         results.append((parts[0], opts))
 
     return results

pb.h

@@ -50,7 +50,7 @@
 
 /* Version of the nanopb library. Just in case you want to check it in
  * your own program. */
-#define NANOPB_VERSION nanopb-0.3.2
+#define NANOPB_VERSION nanopb-0.3.3-dev
 
 /* Include all the system headers needed by nanopb. You will need the
  * definitions of the following:

pb_decode.c

@@ -396,6 +396,10 @@ static bool checkreturn decode_static_field(pb_istream_t *stream, pb_wire_type_t
 
         case PB_HTYPE_ONEOF:
             *(pb_size_t*)iter->pSize = iter->pos->tag;
+            if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE)
+            {
+                pb_message_set_to_defaults((const pb_field_t*)iter->pos->ptr, iter->pData);
+            }
             return func(stream, iter->pos, iter->pData);
 
         default:

tests/oneof/decode_oneof.c

@@ -2,22 +2,101 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <pb_decode.h>
 #include "oneof.pb.h"
 #include "test_helpers.h"
 #include "unittests.h"
 
+/* Test the 'OneOfMessage' */
+int test_oneof_1(pb_istream_t *stream, int option)
+{
+    OneOfMessage msg;
+    int status = 0;
+
+    /* To better catch initialization errors */
+    memset(&msg, 0xAA, sizeof(msg));
+
+    if (!pb_decode(stream, OneOfMessage_fields, &msg))
+    {
+        printf("Decoding failed: %s\n", PB_GET_ERROR(stream));
+        return 1;
+    }
+
+    /* Check that the basic fields work normally */
+    TEST(msg.prefix == 123);
+    TEST(msg.suffix == 321);
+
+    /* Check that we got the right oneof according to command line */
+    if (option == 1)
+    {
+        TEST(msg.which_values == OneOfMessage_first_tag);
+        TEST(msg.values.first == 999);
+    }
+    else if (option == 2)
+    {
+        TEST(msg.which_values == OneOfMessage_second_tag);
+        TEST(strcmp(msg.values.second, "abcd") == 0);
+    }
+    else if (option == 3)
+    {
+        TEST(msg.which_values == OneOfMessage_third_tag);
+        TEST(msg.values.third.array[0] == 1);
+        TEST(msg.values.third.array[1] == 2);
+        TEST(msg.values.third.array[2] == 3);
+        TEST(msg.values.third.array[3] == 4);
+        TEST(msg.values.third.array[4] == 5);
+    }
+
+    return status;
+}
+
+
+/* Test the 'PlainOneOfMessage' */
+int test_oneof_2(pb_istream_t *stream, int option)
+{
+    PlainOneOfMessage msg = PlainOneOfMessage_init_zero;
+    int status = 0;
+
+    if (!pb_decode(stream, PlainOneOfMessage_fields, &msg))
+    {
+        printf("Decoding failed: %s\n", PB_GET_ERROR(stream));
+        return 1;
+    }
+
+    /* Check that we got the right oneof according to command line */
+    if (option == 1)
+    {
+        TEST(msg.which_values == OneOfMessage_first_tag);
+        TEST(msg.values.first == 999);
+    }
+    else if (option == 2)
+    {
+        TEST(msg.which_values == OneOfMessage_second_tag);
+        TEST(strcmp(msg.values.second, "abcd") == 0);
+    }
+    else if (option == 3)
+    {
+        TEST(msg.which_values == OneOfMessage_third_tag);
+        TEST(msg.values.third.array[0] == 1);
+        TEST(msg.values.third.array[1] == 2);
+        TEST(msg.values.third.array[2] == 3);
+        TEST(msg.values.third.array[3] == 4);
+        TEST(msg.values.third.array[4] == 5);
+    }
+
+    return status;
+}
+
 int main(int argc, char **argv)
 {
     uint8_t buffer[OneOfMessage_size];
-    OneOfMessage msg = OneOfMessage_init_zero;
-    pb_istream_t stream;
     size_t count;
     int option;
 
     if (argc != 2)
     {
-        fprintf(stderr, "Usage: encode_oneof [number]\n");
+        fprintf(stderr, "Usage: decode_oneof [number]\n");
         return 1;
     }
     option = atoi(argv[1]);
@@ -31,42 +110,22 @@ int main(int argc, char **argv)
         return 1;
     }
 
-    stream = pb_istream_from_buffer(buffer, count);
-
-    if (!pb_decode(&stream, OneOfMessage_fields, &msg))
-    {
-        printf("Decoding failed: %s\n", PB_GET_ERROR(&stream));
-        return 1;
-    }
-
     {
         int status = 0;
+        pb_istream_t stream;
 
-        /* Check that the basic fields work normally */
-        TEST(msg.prefix == 123);
-        TEST(msg.suffix == 321);
+        stream = pb_istream_from_buffer(buffer, count);
+        status = test_oneof_1(&stream, option);
 
-        /* Check that we got the right oneof according to command line */
-        if (option == 1)
-        {
-            TEST(msg.which_values == OneOfMessage_first_tag);
-            TEST(msg.values.first == 999);
-        }
-        else if (option == 2)
-        {
-            TEST(msg.which_values == OneOfMessage_second_tag);
-            TEST(strcmp(msg.values.second, "abcd") == 0);
-        }
-        else if (option == 3)
-        {
-            TEST(msg.which_values == OneOfMessage_third_tag);
-            TEST(msg.values.third.array[0] == 1);
-            TEST(msg.values.third.array[1] == 2);
-            TEST(msg.values.third.array[2] == 3);
-            TEST(msg.values.third.array[3] == 4);
-            TEST(msg.values.third.array[4] == 5);
-        }
+        if (status != 0)
+            return status;
 
-        return status;
+        stream = pb_istream_from_buffer(buffer, count);
+        status = test_oneof_2(&stream, option);
+
+        if (status != 0)
+            return status;
     }
-}
+
+    return 0;
+}

tests/oneof/oneof.proto

@@ -5,6 +5,7 @@ message SubMessage
     repeated int32 array = 1 [(nanopb).max_count = 8];
 }
 
+/* Oneof in a message with other fields */
 message OneOfMessage
 {
     required int32 prefix = 1;
@@ -16,3 +17,14 @@ message OneOfMessage
     }
     required int32 suffix = 99;
 }
+
+/* Oneof in a message by itself */
+message PlainOneOfMessage
+{
+    oneof values
+    {
+        int32 first = 5;
+        string second = 6 [(nanopb).max_size = 8];
+        SubMessage third = 7;
+    }
+}

tests/options/options.proto

@@ -75,3 +75,12 @@ message SkippedMessage
     option (nanopb_msgopt).skip_message = true;
     required int32 foo = 1;
 }
+
+// Message with oneof field
+message OneofMessage
+{
+    oneof foo {
+        int32 bar = 1;
+    }
+}
+

tools/make_linux_package.sh

@@ -31,7 +31,8 @@ rm $DEST/generator/protoc-gen-nanopb.py
 # Package the protoc compiler
 cp `which protoc` $DEST/generator-bin/protoc.bin
 LIBPROTOC=$(ldd `which protoc` | grep -o '/.*libprotoc[^ ]*')
-cp $LIBPROTOC $DEST/generator-bin/
+LIBPROTOBUF=$(ldd `which protoc` | grep -o '/.*libprotobuf[^ ]*')
+cp $LIBPROTOC $LIBPROTOBUF $DEST/generator-bin/
 cat > $DEST/generator-bin/protoc << EOF
 #!/bin/bash
 SCRIPTDIR=\$(dirname "\$0")