Lint warnings

[GoogleCodeExporter]

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.  

Original issue reported on code.google.com by jone...@jonesmz.com on 31 Aug 2012 at 8:48

[GoogleCodeExporter]

The warnings from the generated code may be difficult to fix. In a way, lint is 
correct, because the pointer hacks in the pb_field_t are quite intricate.

On the other hand the warnings from main nanopb code should probably be fixed. 
I'm still marking this as enhancement, as there is no functional nor 
compilation problem caused because of these.

Original comment by Petteri.Aimonen on 1 Sep 2012 at 7:53

• Changed state: Accepted
• Added labels: Priority-Low, Type-Enhancement
• Removed labels: Priority-Medium, Type-Defect

[GoogleCodeExporter]

Hmm, what lint (splint?) settings are you using, if you get so few warnings? 
With the default settings, splint spits a screenfull of useless stuff at me:

pb_decode.c:464:136: Only storage iter.current (type pb_field_t *) derived from
    variable declared in this scope is not released (memory leak)
pb_decode.c:464:136: Only storage iter.dest_struct (type void *) derived from
    variable declared in this scope is not released (memory leak)
pb_decode.c:464:136: Only storage iter.pData (type void *) derived from
    variable declared in this scope is not released (memory leak)
pb_decode.c:464:136: Only storage iter.pSize (type void *) derived from
    variable declared in this scope is not released (memory leak)

I'm not going to litter the code with lint directives. And I can see nothing 
wrong with most of the splint warnings and no way to change the code to avoid 
them. Heck, a memory leak in a function that doesn't allocate anything, ever?

I fixed the pb_encode.c:272 (uint64_t) cast. It is there only because someone 
reported that his compiler didn't automatically convert 32-bit types to 64-bit 
types when used as function arguments.

Regarding the & before function names to take a pointer: in my opinion it is 
much clearer to use & to mark a function pointer. Therefore I'm not going to 
change it just for splint.

For now, I don't think splint is that useful for checking nanopb code. I of 
course welcome any patches that cleanly avoid the warnings.

Original comment by Petteri.Aimonen on 2 Sep 2012 at 5:22

[GoogleCodeExporter]

im not really sure what lint / splint settings we use. i think that a lot of 
times lint gives bogus warnings anyway. I dont see any problem with you marking 
tjis as wont fix, we can just turn off lint for nanopb.

thanks!

Original comment by jone...@jonesmz.com on 2 Sep 2012 at 5:28

[GoogleCodeExporter]

Original comment by Petteri.Aimonen on 2 Sep 2012 at 5:45

• Changed state: WontFix