What steps will reproduce the problem?
1. Compile the code

What is the expected output? What do you see instead?

I expect the code to compile cleanly.  I see warnings like 

"conversion from __int64 to int, possible loss of data"

What version of the product are you using? On what operating system?

npb-0.1.6.  Windows.

Please provide any additional information below.

Patch attached.

The pb_dec_bytes, pb_dec_string and pb_dec_submessage functions are probably 
unnecessary for any library users, and should be hidden from the public 
interface.

The pb_dec_varint, pb_dec_svarint etc. would be easier to use if there were 
small wrapper functions around them. Should probably write the wrappers to 
pb_decode.h as inline/static functions, so they don't take up code space.

nanopb works for me, but the following code is suspicious in pb_decode.c:


pb_istream_t pb_istream_from_buffer(uint8_t *buf, size_t bufsize)
{
    pb_istream_t stream;
...
    return stream;
}

since pb_istream is allocated on the stack, referring to it upstream for struct 
assignment of the return value depends on the compiler/runtime support leaving 
the stack intact, which might be a hit-and-miss game

Suggested change:

typedef struct _pb_istream_t *pb_istream_ptr_t;

void pb_istream_from_buffer(pb_istream_ptr_t *stream, uint8_t *buf, size_t 
bufsize)
{
    stream->callback = &buf_read;
    stream->state = buf;
    stream->bytes_left = bufsize;
}

Usage:

pb_istream_t foo_stream;
pb_istream_from_buffer(&foo_stream, buf, size):

flag -Wlogical-op is not supported by clang

On platforms with small memory, it is useful to pack the nanopb generated 
structures. The GCC option -fpack-struct would disturb also 3rd party code.

Add generator option (nanopb).packed_struct that will place the `pb_packed` 
macro after each generated structure.

What steps will reproduce the problem?
1. Create a message that has a repeated field followed by a bytes field
2. Send the message with data in both fields 
3.

What is the expected output? What do you see instead?
Both fields will be decoded. Only the repeated field is decoded, then an error 
is reported.

What version of the product are you using? On what operating system?
0.1.4

Please provide any additional information below.
In pb_decode.c::decode_field(), shouldn't there be something like

stream->state = substream.state;

before the "case PB_HTYPE_ARRAY" returns?

I'm compiling on Windows, using a Microsoft compiler set to use the  highest 
warning level.  The code generates quite a few warnings.

Not a very exciting bug - but it would be nice to clean this up.

Attached a patch that resolves all warnings on my machine.  Patch is against 
the nanopb-0.1.1 release.   

Thanks!

David

Can you please add a comment in the generated files xx.pb.c/.h with the version 
of nanopb that was used to generate them?
It would enhance local traceability for end-user.
seen on nanopb-0.1.5

Filenames such as foo-bar.proto will cause generator to put #ifdef 
__FOO-BAR_PB_H__ in the header. Compilation of this fails with:

bc-alltypes.pb.h:2:15: error: extra tokens at end of #ifndef directive [-Werror]

The API reference on "Decoding fields manually" says:

"For decoding strings and bytes fields, the length has already been decoded. 
You can therefore check the total length in stream->state and read the data 
using pb_read."

As far as I can tell, the length is stored in stream->bytes_left and not 
stream->state.

$ make -C example
...
client.c:33:5: error: format ‘%lld’ expects argument of type ‘long long 
int’, but argument 2 has type ‘uint64_t’ [-Werror=format]
...
$

Cast'ing integer to uintmax_t and using %ju instead fixes this problem.

nanopb 0.1.2

Add a note that the stream read callbacks should always read the specified 
number of bytes, unless there is error or EOF. For example recv() needs the 
MSG_WAITALL parameter, as used in the example.

With some toolchains, it may be useful to build nanopb using a C++ compiler. 
This may however give errors, as in C it is ok to implicitly cast void* to 
another pointer.

Adding a few casts here and there will fix this. Also need to add a testcase 
that compiles pb_encode.c and pb_decode.c using g++.

Some compiler's do not like |-operator applied on enum values. 

Stan Hu has prepared a patch that fixes this:
http://code.google.com/r/stanhu-nanopb-enum-warnings-fixes/source/detail?r=6105b
9422c62d29dcb0a3dcdc27a953247508482

TODO: Check that it works, merge it to master.

I've encountered a few warnings emitted by the tool Lint when running it 
against pb_encode.c and also against generated files.

    &pb_enc_varint,
pb_encode.c  25  Warning 546: Suspicious use of &
                   _
From this page: http://legacy.cleanscape.net/products/cpp/checks.html

This warning is: An attempt was made to take the address of a function name. 
Since names of functions by themselves are promoted to address, the use of the 
& is redundant and could be erroneous.

Same here:

    stream.callback = &buf_write;
pb_encode.c  48  Warning 546: Suspicious use of &



In the function pb_encode_tag

    return pb_encode_varint(stream, (uint64_t)tag);
pb_encode.c  272  Warning 571: Suspicious cast

I'm curious why you don't declare tag to be a uint64_t instead of an int?


I also get a lot of warnings about the generated .c files:

const pb_field_t SystemLoad_fields[3] = {
    {1, (pb_type_t) ((int) PB_HTYPE_REQUIRED | (int) PB_LTYPE_VARINT),
    offsetof(SystemLoad, timestamp), 0,
    pb_membersize(SystemLoad, timestamp), 0, 0},

    {2, (pb_type_t) ((int) PB_HTYPE_REQUIRED | (int) PB_LTYPE_STRING),
    pb_delta_end(SystemLoad, sysload, timestamp), 0,
    pb_membersize(SystemLoad, sysload), 0, 0},

    PB_LAST_FIELD
};

Gives me: 
#... _builtin_offsetof (SystemLoad, timestamp)
    offsetof(SystemLoad, timestamp), 0,
logmessage.pb.c  157  Error 78: Symbol 'SystemLoad' typedef'ed at line 54, file
    logmessage.pb.h, module send_sys_status_event.c used in expression
                                  _
    offsetof(SystemLoad, timestamp), 0,
logmessage.pb.c  157  Warning 516: Symbol '__builtin_offsetof()' has arg. type
    conflict (arg. no. 1 -- struct vs. struct) with line 8
logmessage.pb.c  157  Warning 530: Symbol 'SystemLoad' (line 54, file
    logmessage.pb.h, module send_sys_status_event.c) not initialized
                                               _
#...             __builtin_offsetof (SystemLoad, sysload)
#...              (offsetof(SystemLoad, sysload) - offsetof(SystemLoad, timesta
    pb_delta_end(SystemLoad, sysload, timestamp), 0,
logmessage.pb.c  161  Error 78: Symbol 'SystemLoad' typedef'ed at line 54, file
    logmessage.pb.h, module send_sys_status_event.c used in expression
                                               _
#...            (offsetof(SystemLoad, sysload) - offsetof(SystemLoad, timestamp
    pb_delta_end(SystemLoad, sysload, timestamp), 0,
logmessage.pb.c  161  Warning 516: Symbol '__builtin_offsetof()' has arg. type
    conflict (arg. no. 1 -- struct vs. struct) with line 8
                                               _
#...             __builtin_offsetof (SystemLoad, timestamp)
#...  sysload) - offsetof(SystemLoad, timestamp) - pb_membersize(SystemLoad, ti
    pb_delta_end(SystemLoad, sysload, timestamp), 0,
logmessage.pb.c  161  Error 78: Symbol 'SystemLoad' typedef'ed at line 54, file
    logmessage.pb.h, module send_sys_status_event.c used in expression
                                               _
#... ysload) - offsetof(SystemLoad, timestamp) - pb_membersize(SystemLoad, time
    pb_delta_end(SystemLoad, sysload, timestamp), 0,
logmessage.pb.c  161  Warning 516: Symbol '__builtin_offsetof()' has arg. type
    conflict (arg. no. 1 -- struct vs. struct) with line 8



I'm not really sure what code changes could be used to prevent lint from 
complaining about these. I'll probably just suppress them in my project.

Thanks for the fantastic project.  

The nanopb generator currently requires separately installing Google protobuf 
libraries and the protoc compiler. It might make sense to create installation 
packages for the nanopb generator which would contain all dependencies.

It would be nice to provide an example with callback usage. Available examples 
are too simple to demonstrate nanopb's functionality.

What steps will reproduce the problem?
1. Generate code using the tests\options.proto sample file

What is the expected output? What do you see instead?
The enum type is generated incorrectly.  The name is missing:

/* Enum definitions */
typedef enum _ {
    EnumValue1 = 1
} ;

What version of the product are you using? On what operating system?
0.1.7 on Windows

What steps will reproduce the problem?

$ make -C example
...
common.c: In function ‘write_callback’:
common.c:13:14: error: cast from pointer to integer of different size 
[-Werror=pointer-to-int-cast]
...
$

GCC complains about void* <-> int conversion. Converting through intptr_t fixes 
this problem.

nanopb 0.1.2

If I declare a field as:                                                        


optional bytes iv=3 [(nanopb).max_size = 16];                                   


the pb_encode() function does not take the size member of the                   

pb_bytes_array_t struct into account when                                       

it updates the pData pointer on line 155-158 in pb_encode.c                     


It is caused by the pb_generator.py line 241-242                                


elif self.htype != 'PB_HTYPE_CALLBACK' and not self.is_pointer and              

self.ltype == 'PB_LTYPE_BYTES':                                                 

result += '\n    pb_membersize(%s, bytes),' % self.ctype                        


only measuring the size of the bytes array and not including the size member.   


I have solved this temporarily by updating the generated field                  

descriptor manually to measure the                                              

size of the whole pb_bytes_array_t struct but I am not sure it has              

other side effects...

What steps will reproduce the problem?
1. create some message that has nested submessages. E.g.

message Leaf {
   required float someValue = 1;
}

message Root {
   required Leaf submessage = 1;
}


2. call pb_decode() for the Root structure.
3. pb_message_set_to_defaults is called first - the in walks recursively to all 
nested substructures and fields to set default values (like 0)
4. Then when going further step-by-step witin pb_decode(), submessage is being 
decoded (recursion) and pb_message_set_to_defaults is being called again (on 
submessage level) and then again it initializes fields of Leaf strucutre. 

So before field values are actually decoded and set, they are set to defaults 
twice.

If we would have message with 3-levels of nested submessages, this overhead 
would cause having given fields being initialized three times without a need.

Since it's minor overhead for messages without nesting, becomes large when 
dealing with heavily nested structures with larger strings or bytes fields 
(where memsets take a time).

What version of the product are you using? On what operating system?
nanopb-0.1.1. (but same code is in latest nanopb-0.1.5 version)
cross-compilation on Windows for ARM Cortex M4.
GCC-4.6 / devkitARM

What steps will reproduce the problem?
1. add import "some_other.proto" to the current proto file
2. run code generator

What is the expected output? What do you see instead?
i expect the generated header file should also include the header file of the 
imported file, but this is not the case. As a result, compilation breaks. 


What version of the product are you using? On what operating system?
0.1.1.

Great job on the project by the way. 

What steps will reproduce the problem?
1. insert generated files in a C++ project
2. from a foo.cpp source file, include the generated xx.pb.h
3. in this cpp file, use the tYyy_fields structure

What is the expected output?
Link should succeed.

What do you see instead?
error LNK2001: external symbol not found "struct _pb_field_t const * const 
tYyy_fields" (?tYyy_fields@@3QBU_pb_field_t@@B)

What version of the product are you using? On what operating system?
Visual Studio 2005 on a Win7

Please provide any additional information below.
This error is due to function decoration error when compiling foo.cpp.
To avoid this, you can maybe add the following in the generated xx.pb.h to 
force a "C" style decoration.
    #ifdef __cplusplus
        extern "C"
        {
    #endif
            /* Struct field encoding specification for nanopb */
            extern const pb_field_t tJointState_fields[3];
            /* ... */
    #ifdef __cplusplus
        extern "C"
        {
    #endif

This would be an enhancement as another solution is to use in the foo.cpp file 
the following lines:
extern "C"
{
    #include "xx.pb.h"
}

Hi!

I have done one test as goes wrong.

doppio:~/tmp/nanopb/rkm> ./rkm
ERROR endian_copy #284 destsize:8 srcsize:16 EXIT
doppio:~/tmp/nanopb/rkm> 

Is this way to use the packet not OK?

In dir rkm you find my test and I have add some code lines in the pb_encode.c:

-----------------------pb_encode.c----------------------
static void endian_copy(void *dest, const void *src, size_t destsize, size_t 
srcsize)
{
    if (destsize < srcsize)
    {
        fprintf(stderr, "ERROR %s #%d destsize:%zu srcsize:%zu EXIT\n",__func__,__LINE__,destsize, srcsize);
        exit(1);
    }
.....
---------------------

MvH/[email protected]

PS: I run on a 64 bit ubuntu 10.04

What steps will reproduce the problem?
1.
Define message like this:

import "nanopb.proto";
message msg {
  required string string1       = 2 [(nanopb).max_size = 256];
  required string String2       = 3 [(nanopb).max_size = 256];
}

2.
Observe that the value of msg_fields[].data_size will be initialized to zero by 
the macro function pb_membersize(msg, string1) (or string2). This happens 
because data_size is a uint8_t, and setting it to the value 256 is an overflow, 
which rolls data_size back to 0.

3.
The resulting behavior is that when msg is encoded, string1 is encoded twice, 
instead of string1 being encoded, followed by string2. This follows because 
when we get to

bool checkreturn pb_encode(pb_ostream_t *stream, const pb_field_t fields[], 
const void *src_struct)
{
    const pb_field_t *field = fields;
    const void *pData = src_struct;
    const void *pSize;
    size_t prev_size = 0;

    while (field->tag != 0)
    {
        pb_encoder_t func = PB_ENCODERS[PB_LTYPE(field->type)];
        pData = (const char*)pData + prev_size + field->data_offset;
        pSize = (const char*)pData + field->size_offset;

        prev_size = field->data_size;


prev_size is set to 0, and the subsequent iteration through the loop doesn't 
move the pointer to the data, and so we simply encode string1 again.


What is the expected output? What do you see instead?
I'd expect one of two possibilities.

One possibility is that nanopb should work correctly with strings of size 256 
or larger. To do this, I suspect you'll have to increase the size of uint8_t 
data_size; Maybe make it a size_t instead of a uint8_t. 

The second possibility is that when generating the .c and .h files from the 
.proto/.pb files, the generator code could emit a warning saying that we'll get 
weird behavior. 

What version of the product are you using? On what operating system?
Scientific Linux 6.2 (32 bit), and I'm using release 0.1.3.

Please provide any additional information below.
We aren't using nanopb in a memory constrained environment, so we don't really 
mind the extra memory overhead of using size_t. One thought is to define 
#define pb_size_type, and default to size_t (or uint8_t, either way), but let 
the end user pass a compiler flag -DUSE_SMALL_SIZE_TYPE to switch pb_size_type 
from size_t to uint8_t, for those users who have memory constrained 
environments.

When using an old gnu compiler, with all warnings displayed, the define 
checkreturn __attribute__((warn_unused_result)) is not supported. It generates 
following messages during compilation:
warning: `warn_unused_result' attribute directive ignored

To solve this, please introduce additional conditions around checkreturn 
definition like:

/* The warn_unused_result attribute appeared first in gcc-3.4.0 */
#if !defined(__GNUC__) || ( __GNUC__ < 3) || (__GNUC__ == 3 && __GNUC_MINOR__ < 
4)
    #define checkreturn
#else
    /* Verify that we remember to check all return values for proper error propagation */
    #define checkreturn __attribute__((warn_unused_result))
#endif

Header files generated by nanopb declare all structs as types, e.g. "typedef 
struct {} foo;". This makes forward declaration of them hard to impossible, and 
a struct name would be desirable. Example: "typedef struct foo_s {} foo;"

When trying to compile files generated with nanopb-0.1.5 (myFile.pb.c), I get 
error of missing <string> and <cstddef> files included by  
src\google\protobuf\descriptor.pb.h directory.
These are C++ libraries, not available in ANSI-C.

Problem was not existing with nanopb-0.1.1. 
So I guess somewhere between 0.1.1 and 0.1.5 nanopb lost declared ANSI-C 
compliance.

Dear Petteri,

I’m trying to get nanopb running but face a problem with REPEATED fields.

If I use your static extension I get problems when trying to compile the proto 
file for the PC into a Google C++ file.

It generates this function:

void protobuf_AddDesc_nanopb_2eproto() {
  static bool already_here = false;
  if (already_here) return;
  already_here = true;
  GOOGLE_PROTOBUF_VERIFY_VERSION;

  ::google::protobuf::protobuf_AddDesc_generator_2fdescriptor_2eproto();
  ::google::protobuf::DescriptorPool::InternalAddGeneratedFile(
    "\n\014nanopb.proto\032\032generator/descriptor.pro"
    "to\"4\n\rNanoPBOptions\022\020\n\010max_size\030\001 \001(\005\022\021\n"
    "\tmax_count\030\002 \001(\005:>\n\006nanopb\022\035.google.prot"
    "obuf.FieldOptions\030\362\007 \001(\0132\016.NanoPBOptions", 160);
  ::google::protobuf::MessageFactory::InternalRegisterGeneratedFile(
    "nanopb.proto", &protobuf_RegisterTypes);
  NanoPBOptions::default_instance_ = new NanoPBOptions();
  ::google::protobuf::internal::ExtensionSet::RegisterMessageExtension(
    &::google::protobuf::FieldOptions::default_instance(),
    1010, 11, false, false,
    &::NanoPBOptions::default_instance());
  NanoPBOptions::default_instance_->InitAsDefaultInstance();
  ::google::protobuf::internal::OnShutdown(&protobuf_ShutdownFile_nanopb_2eproto);
}



And I get the compile error

Error      1             error C2039: 
'protobuf_AddDesc_generator_2fdescriptor_2eproto' : is not a member of 
'google::protobuf'                
C:\Entwicklung\WaveLabs\Software\PC-Client\emUSBClient\nanopb.pb.cc    76       
    1             emUSBClient
Error      2             error C3861: 
'protobuf_AddDesc_generator_2fdescriptor_2eproto': identifier not found         
       C:\Entwicklung\WaveLabs\Software\PC-Client\emUSBClient\nanopb.pb.cc    
76           1             emUSBClient


If I don’t use your extension the C++ files are ok, but I have problems to 
fully understand the use of the call backs:

When encoding REPEATED fields, am I right that the callback is called only once 
and I have to write all fields at once?
Do I have to write the tag for each field?
I have understood your example for simples types, but I have a repeated field 
that itself is a submessage (.proto is attached) who do write the call back for 
that?
Also how to I know when decoding repeated fields with the call back how many 
fields are transmitted?


Because of the problems with the callback I would prefer to use the static 
method but I have to solve the C++ problems.

I hope you can help me.

Best
Thomas

doppio:~/tmp/nanopb/nanopb-0.1.2/tests> make
protoc -I. -I../generator -I/usr/include -operson.pb person.proto
python ../generator/nanopb_generator.py person.pb
Traceback (most recent call last):
  File "../generator/nanopb_generator.py", line 4, in <module>
    import nanopb_pb2
  File "/home/roger/tmp/nanopb/nanopb-0.1.2/generator/nanopb_pb2.py", line 10, in <module>
    DESCRIPTOR = descriptor.FileDescriptor(
AttributeError: 'module' object has no attribute 'FileDescriptor'
make: *** [person.pb.h] Error 1

doppio:~/tmp/nanopb/nanopb-0.1.2/tests> uname -a
Linux doppio 2.6.32-38-generic #83-Ubuntu SMP Wed Jan 4 11:12:07 UTC 2012 
x86_64 GNU/Linux
doppio:~/tmp/nanopb/nanopb-0.1.2/tests> 

doppio:~/tmp/nanopb/nanopb-0.1.2/tests> protoc --version
libprotoc 2.2.0

doppio:~/tmp/nanopb/nanopb-0.1.2/tests> python --version
Python 2.6.5

What steps will reproduce the problem?
1.Before we pack the data, there is no way that we can get the packed-size. 
protobuf-c has xxxxx__get_packed_size() api, which gives the packed-size. Can 
we have similar api in nanopb?
2.
3.

What is the expected output? What do you see instead?


What version of the product are you using? On what operating system?


Please provide any additional information below.

Check that the 'extension' construct in .proto files works correctly.

Need to figure out a good way to handle extensions that are defined in a 
separate file.

The issue for me is that my platform's compiler errors when I try               

to initialize an array with non-constant values. It has some ANSI C-isms;       

this is one of them. I did something similar with the decode side; just         

checking because the encode changed in this release.

--- pb_encode.c.orig    2012-06-13 09:32:36.443881585 -0700                     

+++ pb_encode.c 2012-06-13 09:35:59.816378851 -0700                             

@@ -235,8 +235,15 @@                                                            

 bool checkreturn pb_encode_fixed32(pb_ostream_t *stream, const void *value)                                                                
 {                                                                                                                                          
     #ifdef __BIG_ENDIAN__                                                                                                                  
-    uint8_t *bytes = value;                                                    

-    uint8_t lebytes[4] = {bytes[3], bytes[2], bytes[1], bytes[0]};             

+    // Modifications begin                                                     

+    //   Description: compiler is picky about array initialization from        

variables                                                                       

+    uint8_t *bytes = (uint8_t *) value;                                        

+    uint8_t lebytes[4];                                                        

+    lebytes[0] = bytes[3];                                                     

+    lebytes[1] = bytes[2];                                                     

+    lebytes[2] = bytes[1];                                                     

+    lebytes[3] = bytes[0];                                                     

+    // Modifications end                                                       

     return pb_write(stream, lebytes, 4);            

What steps will reproduce the problem?

My message struct:

message msg {
    required string name = 1 [(nanopb).max_size = 16];
    required int32 cmd = 2;
    required int32 start = 3;
    required int32 len = 4;
    optional bytes data = 5 [(nanopb).max_size = 512];
}

When msg.data.size = 512 - pb_decode do not decode message. for example 
msg.data.size = 256 work nice!

What version of the product are you using? On what operating system?

Last repositary wersion

While experimenting with different ways to structure my data I encountered 
serious problems if I used to repeated elements in one Message. e.g.

Works:

message TestArray{
    required MessageType type = 1;
    repeated uint32 light_u = 2 [(nanopb).max_count = 410]; 
}

Sizeof TestArray: 1648 (only 40 elements are initialized)
Coded Size: 124


Doesn't work:

message TestArray2{
    required MessageType type = 1;
    repeated uint32 light_i = 2 [(nanopb).max_count = 410]; 
    repeated uint32 light_u = 3 [(nanopb).max_count = 410]; 
}

Sizeof TestArray2: 3292
Coded Size: 5997

Where as Codes Size is always one byte less than the used buffer.


Arrays inside a Submessage produce even stranger results:


message MeassurementPlainArray {
    required uint32 cellID = 1;
    repeated uint32 light_i = 2 [(nanopb).max_count = 410];
    repeated uint32 light_u = 3 [(nanopb).max_count = 410]; 
}


message USBMessagePlainArray{
    required MessageType type = 1;
    optional MeassurementPlainArray meassurement = 2;
}


Sizeof USBTestMessagePlainArray: 3300
Coded Size: 6



I have attached the .proto File and the Test sources.

Best
Thomas

1.My structure have string item with max length 1 byte.
2.I shuld make the second byte to store the null

may be add byte field type and word field type?

Sometimes it may be useful to force a required or optional field to be of 
callback type. This can be kludged using "repeated" instead, but that's not 
very clean.

Implement (nanopb).type option in the .proto file, that has values "default", 
"static" and "callback". Take note of the similar implementation in the 
dynamic-alloc branch.

Currently nanopb returns just `false` when any error of any kind occurs. Even 
though many applications likely treat all errors in the same way (i.e. it 
doesn't matter if it is an IO error or an invalid message), some applications 
would benefit from more detailed errors. More importantly, it would make 
debugging a lot easier to have atleast a rough idea of what causes an error.

Error codes would not increase code size by much, but they will break the 
current interface. Whereas `if (!pb_encode(..)) { error }` works now, it would 
be the exact opposite when following the convention that PB_OK = 0 and 
PB_READ_ERROR = 1 etc.

Therefore this bug is postponed until some major release.

Compiling pb_decode.c with g++ (not gcc) -Wall results in a few casting 
warnings.  Here's one:

pb_decode.c: In function ‘void pb_message_set_to_defaults(const pb_field_t*, 
void*)’:
pb_decode.c:394:69: error: invalid conversion from ‘const void*’ to 
‘const pb_field_t* {aka const _pb_field_t*}’ [-fpermissive]
pb_decode.c:396:13: error:   initializing argument 1 of ‘void 
pb_message_set_to_defaults(const pb_field_t*, void*)’ [-fpermissive]

I made a few changes here:

http://code.google.com/r/stanhu-gplusplus-warning-fixes/source/diff?spec=svn07a2
276b838cbf6f59b55e595f4cac70afd24e29&r=07a2276b838cbf6f59b55e595f4cac70afd24e29&
format=side&path=/pb_decode.c

What steps will reproduce the problem?
1. Create a proto file importing another proto file.
2. Compile with clang or gcc

See change: 
http://code.google.com/r/steffensiering-clang-and-static-assert-fixes/source/det
ail?r=afbffdc46fb461448680f4cd401144315e3c2dc6

What is the expected output? What do you see instead?
Does not compile due to redefinition of typedef. Both clang and gcc

After upgrading from nanopb-0.1.1 to nanopb-0.1.2, generated foo.bp.h includes 
"google/protobuf/descriptor.pb.h":

#include "google/protobuf/descriptor.pb.h"

descriptor.pb.h (which is in protobuf) includes <string> -- C++ file. This 
breaks the build process. Manually removing line #include 
"google/protobuf/descriptor.pb.h" seems to fix the problem. Is this include 
really needed, since it's supposed to be clean C build?

It would be extremely useful if nanopb could support enums defined within a 
message:

message foo
{
   required string name = 1;
   enum bar
   {
      first = 0;
      second = 1;
   }
   optional bar value = 2;
}

What steps will reproduce the problem?
1. Modify the tests\options.proto sample file and add the following to the 
Messasge4 message:
required Enum1 enum = 2 [default = EnumValue1];

2. Generate code

What is the expected output? What do you see instead?
Expected: const Enum1 Message4_enum_default = EnumValue1;
Actual: const Enum1 Message4_enum_default = Enum1_EnumValue1;


What version of the product are you using? On what operating system?
0.1.7 on Windows

Currently pb_decode() only detects missing required fields for messages of less 
than 32 fields.

There are many ways to implement this; dynamic-alloc branch uses a bitfield 
stored in the message structure, but that requires somewhat ugly macros for 
handling the bitfield.

Instead, a less intrusive implementation plan:

1) Add required_field_index to pb_field_iterator_t. Increment it for every 
required field. Use it to set bit in fields_seen just for PB_HTYPE_REQUIRED 
fields.

2) Add a new compile-time option PB_MAX_REQUIRED_FIELDS, with default value 32.

3) Make the fields_seen bitfield in pb_decode as uint8_t 
fields_seen[PB_MAX_REQUIRED_FIELDS / 8].

4) Make the generator generate code that checks for sufficient limit:
#if PB_MAX_REQUIRED_FIELDS < 45
#warning Message FooBarMessage requires PB_MAX_REQUIRED_FIELDS >= 45 to 
properly check for missing required fields.
#endif

Thanks to Greg Klein for report and help in designing the fix.

What steps will reproduce the problem?
1. Attempt to decode a message where you will skip a String field (set callback 
to NULL)
2. Run pb_decode() using an input stream whose callback implementation matches 
the implementation in test_decode2.c
3. Observe IO error

What is the expected output? What do you see instead?
I would expect it to skip the field correctly. Instead, the input stream 
callback erroneously returns false because the last decrement on the 'count' 
argument wrapped it around to the largest unsigned value for size_t.


What version of the product are you using? On what operating system?
nanopb 0.1.6
Mac OS X 10.8.2


Please provide any additional information below.

The example input stream callback should decrement the 'count' arg in the body 
of the loop rather than in the loop's test expression. For instance:
  if (buf == NULL)
  {
    /* Skipping data */
    while (count && fgetc(file) != EOF)
      count--;
    return count == 0;
  }

It might be nice to provide an optional C++ interface for the generated 
structures. These would be mostly simple wrapper methods, like

void set_foo(int value) { foo = value; has_foo = true; }

This would provide easier porting to/from Google-official C++ interface.

It should probably be enabled with an option to the generator, in order not to 
pollute the C headers with unnecessary #ifdefs.

In protocol buffers, according to protoc.exe (win32 binary), 

"... enum values use C++ scoping rules, meaning that enum values are siblings 
of their type, not children of it."


For the following test.proto:

/////////////////////////////////////////////////////////////////////
enum ENUM_ONE
{
    FLAG_A = 0;
    FLAG_B = 1;
    MAX = 2;
}

enum ENUM_TWO
{
    FLAG_A = 0;
    FLAG_B = 1;
    MAX = 2;
}

message TEST
{
    optional ENUM_ONE one = 1;
    optional ENUM_TWO two = 2;
}
/////////////////////////////////////////////////////////////////////



Compiling with protoc results in this output:

test.proto:12:9: "FLAG_A" is already defined.
test.proto:12:9: Note that enum values use C++ scoping rules, meaning that enum 
values are siblings of their type, not children of it.  Therefore, "FLAG_A" 
must be unique within the global scope, not just within "ENUM_TWO".
test.proto:13:9: "FLAG_B" is already defined.
test.proto:13:9: Note that enum values use C++ scoping rules, meaning that enum 
values are siblings of their type, not children of it.  Therefore, "FLAG_B" 
must be unique within the global scope, not just within "ENUM_TWO".
test.proto:14:9: "MAX" is already defined.
test.proto:14:9: Note that enum values use C++ scoping rules, meaning that enum 
values are siblings of their type, not children of it.  Therefore, "MAX" must 
be unique within the global scope, not just within "ENUM_TWO".


So to resolve this, we use unique names at file scope so all the siblings are 
happy:

/////////////////////////////////////////////////////////////////////
enum ENUM_ONE
{
    ENUM_ONE_FLAG_A     = 0;
    ENUM_ONE_FLAG_B     = 1;
    ENUM_ONE_MAX        = 2;
}

enum ENUM_TWO
{
    ENUM_TWO_FLAG_A     = 0;
    ENUM_TWO_FLAG_B     = 1;
    ENUM_TWO_MAX        = 2;
}

message TEST
{
    optional ENUM_ONE one = 1;
    optional ENUM_TWO two = 2;
}
/////////////////////////////////////////////////////////////////////


Which results in this output in .pb.c file:


/* Enum definitions */
typedef enum {
    ENUM_ONE_ENUM_ONE_FLAG_A = 0,
    ENUM_ONE_ENUM_ONE_FLAG_B = 1,
    ENUM_ONE_ENUM_ONE_MAX = 2
} ENUM_ONE;

typedef enum {
    ENUM_TWO_ENUM_TWO_FLAG_A = 0,
    ENUM_TWO_ENUM_TWO_FLAG_B = 1,
    ENUM_TWO_ENUM_TWO_MAX = 2
} ENUM_TWO;


You can see the enum name prefixes are duplicated.




Suggested patch to nanopb_generator.py:

@@ -69,13 +69,13 @@
     return Names(type_name[1:].split('.'))

 class Enum:
     def __init__(self, names, desc):
         '''desc is EnumDescriptorProto'''
         self.names = names + desc.name
-        self.values = [(x.name, x.number) for x in desc.value]
+        self.values = [(self.names + x.name, x.number) for x in desc.value]

     def __str__(self):
         result = 'typedef enum {\n'
         result += ',\n'.join(["    %s = %d" % x for x in self.values])
         result += '\n} %s;' % self.names
         return result
@@ -125,12 +125,14 @@
         # CTYPE is the name of the c type to use in the struct.
         if datatypes.has_key(desc.type):
             self.ctype, self.ltype = datatypes[desc.type]
         elif desc.type == FieldD.TYPE_ENUM:
             self.ltype = 'PB_LTYPE_VARINT'
             self.ctype = names_from_type_name(desc.type_name)
+            if self.default is not None:
+                self.default = self.ctype + self.default
         elif desc.type == FieldD.TYPE_STRING:
             self.ltype = 'PB_LTYPE_STRING'
             if self.max_size is None:
                 is_callback = True
             else:
                 self.ctype = 'char'

What steps will reproduce the problem?
1. Compile protobug 2.4.1; put "protoc" on path
2. Edit nanopb's example/Makefile, point "-I" on line 13 appropriately
3. Execute commands: cd example; make

What is the expected output? What do you see instead?

Expected clean compile, got errors in Makefile line 14:

python ../generator/nanopb_generator.py fileproto.pb
Traceback (most recent call last):
  File "../generator/nanopb_generator.py", line 408, in <module>
    fdesc = descriptor.FileDescriptorSet.FromString(data)
  File "C:\Python26\lib\site-packages\protobuf-2.4.1-py2.6.egg\google\protobuf\internal\python_message.py", line 533, in FromString
    message.MergeFromString(s)
  File "C:\Python26\lib\site-packages\protobuf-2.4.1-py2.6.egg\google\protobuf\internal\python_message.py", line 755, in MergeFromString
    if self._InternalParse(serialized, 0, length) != length:
  File "C:\Python26\lib\site-packages\protobuf-2.4.1-py2.6.egg\google\protobuf\internal\python_message.py", line 782, in InternalParse
    pos = field_decoder(buffer, new_pos, end, self, field_dict)
  File "C:\Python26\lib\site-packages\protobuf-2.4.1-py2.6.egg\google\protobuf\internal\decoder.py", line 521, in DecodeRepeatedField
    raise _DecodeError('Truncated message.')
google.protobuf.message.DecodeError: Truncated message.
Makefile:13: recipe for target `fileproto.pb.h' failed
make: *** [fileproto.pb.h] Error 1


What version of the product are you using? On what operating system?

nanopb 0.1.1 release
on Windows 7 (64 bit) with cygwin (for shell & make)

Please provide any additional information below.

Atleast MSVC 6.0 errors out on

uint32_t foobar[4] = {};

and similar. Change to use {0}.

When you use the gcc options -Wall and -Wextra, you get this warning.

generated.pb.c:55: warning: missing initializer
generated.pb.c:55: warning: (near initialization for 
‘MessageType_fields[8].data_size’)

The problem is caused by 

#define PB_LAST_FIELD {0,(pb_type_t) 0,0,0}

struct _pb_field_t has 7 fields, so instead the define should be.

#define PB_LAST_FIELD {0,(pb_type_t)0,0,0,0,0,0}