Check splint errors

[claucece]

Why

A library should use a static analyzer.

Issues reported:

in list.c

src/list.c: (in function list_new)
src/list.c:30:12: Null storage returned as non-null: NULL
  Function returns a possibly null pointer, but is not declared using
  /*@null@*/ annotation of result.  If function may return NULL, add /*@null@*/
  annotation to the return value declaration. (Use -nullret to inhibit warning)
src/list.c:36:10: Null storage n->data derivable from return value: n
   src/list.c:33:13: Storage n->data becomes null
src/list.c:36:10: Null storage n->next derivable from return value: n
   src/list.c:34:13: Storage n->next becomes null
src/list.c: (in function otrng_list_foreach)
src/list.c:48:7: Statement has no effect: fn(current, context)
  Statement has no visible effect --- no values are modified. (Use -noeffect to
  inhibit warning)
src/list.c: (in function call_and_free_node)
src/list.c:56:28: Variable fn initialized to type void *, expects [function
                     (void *) returns void] *: context
  Types are incompatible. (Use -type to inhibit warning)
src/list.c:62:3: Implicitly only storage node->data (type void *) not released
                    before assignment: node->data = NULL
  A memory leak has been detected. Only-qualified storage is not released
  before the last reference to it is lost. (Use -mustfreeonly to inhibit
  warning)
src/list.c:63:8: Implicitly temp storage node passed as only param: free (node)
  Temp storage (associated with a formal parameter) is transferred to a
  non-temporary reference. The storage may be released or new aliases created.
  (Use -temptrans to inhibit warning)
src/list.c: (in function otrng_list_free)
src/list.c:67:48: Function otrng_list_foreach expects arg 3 to be void * gets
                     [function (void *) returns void] *: fn
src/list.c:74:19: Function otrng_list_free_nodes inconsistently redeclared to
    return possibly null storage, previously declared without null qualifier
  A function, variable or constant is redefined with a different type. (Use
  -incondefs to inhibit warning)
   src/list.h:45:8: Declaration of otrng_list_free_nodes
src/list.c: (in function otrng_list_free_nodes)
src/list.c:75:25: Null storage passed as non-null param:
                     otrng_list_free (..., NULL)
  A possibly null pointer is passed as a parameter corresponding to a formal
  parameter with no /*@null@*/ annotation.  If NULL may be used for this
  parameter, add a /*@null@*/ annotation to the function parameter declaration.
  (Use -nullpass to inhibit warning)
src/list.c: (in function otrng_list_add)
src/list.c:81:12: Null storage returned as non-null: NULL
src/list.c:84:3: Implicitly only storage n->data (type void *) not released
                    before assignment: n->data = data
src/list.c:84:3: Implicitly temp storage data assigned to implicitly only:
                    n->data = data
src/list.c:86:17: Parse Error. Attempting to continue.
src/list.c:86:17: Cannot recover from parse error.

in tlv.c

src/tlv.c:36:33: Variable TLV_TYPES_LENGTH initialized to type int, expects
                    size_t: OTRNG_TLV_SYM_KEY + 1
  To allow arbitrary integral types to match any integral type, use
  +matchanyintegral.
src/tlv.c: (in function set_tlv_type)
src/tlv.c:41:7: Comparison of unsigned value involving zero: tlv_type >= 0
  An unsigned value is used in a comparison with zero in a way that is either a
  bug or confusing. (Use -unsignedcompare to inhibit warning)
src/tlv.c: (in function parse_tlv)
src/tlv.c:47:49: Null storage passed as non-null param:
                    otrng_tlv_new (..., NULL)
  A possibly null pointer is passed as a parameter corresponding to a formal
  parameter with no /*@null@*/ annotation.  If NULL may be used for this
  parameter, add a /*@null@*/ annotation to the function parameter declaration.
  (Use -nullpass to inhibit warning)
src/tlv.c:49:12: Null storage returned as non-null: NULL
  Function returns a possibly null pointer, but is not declared using
  /*@null@*/ annotation of result.  If function may return NULL, add /*@null@*/
  annotation to the return value declaration. (Use -nullret to inhibit warning)
src/tlv.c:52:9: Parse Error. Attempting to continue.
src/tlv.c:52:9: Cannot recover from parse error.

[olabini]

So, I've just pushed a few comments that makes it possible to run splint in a correct way on ALL our files (except for test files). This gives over 3000 warnings. In order to see them, open the .splintrc file, take away one of the suppressions, and start fixing!

make splint is how to run splint. (obviously you have to have splint installed)

[claucece]

Funny enough, it generates no warnings for me.

[olabini]

Yeah, because I added all the suppressions. As is, it doesn't warn for me either - you have to remove the suppressions in .splintrc like I say in the comment above.

[claucece]

So, yeah @olabini I still get a lot of parsing errors. And I get prepoc errors:

src/ed448.h:24:24: Cannot find include file goldilocks.h on search path:
                      /usr/include;/usr/local/Cellar/splint/3.1.2/include
  Preprocessing error. (Use -preproc to inhibit warning)
   In file included from src/auth.h:26,
                 from src/auth.c:23
src/ed448.h:25:30: Cannot find include file goldilocks/ed448.h on search path:
                      /usr/include;/usr/local/Cellar/splint/3.1.2/include
src/keys.h:24:24: Cannot find include file goldilocks.h on search path:
                     /usr/include;/usr/local/Cellar/splint/3.1.2/include
   In file included from src/auth.h:27,
                 from src/auth.c:23
src/keys.h:25:30: Cannot find include file goldilocks/ed448.h on search path:
                     /usr/include;/usr/local/Cellar/splint/3.1.2/include
src/shake.h:24:30: Cannot find include file goldilocks/shake.h on search path:
    /Users/sofia/repos/otrv4/libotr-ng/src;/usr/include;/usr/local/Cellar/splint
    /3.1.2/include
   In file included from src/auth.c:26

When I add the flag -preproc, it runs. But still gets into parsing errors. This may be another 'cannot fix for MAC'.

[claucece]

So, I sort of scroll on the warnings printed, and this one repeats a lot:

Function parameter a declared as manifest array.. should we fix this?

It is related to:

http://c-faq.com/aryptr/aryptrparam.html https://stackoverflow.com/questions/3655566/what-is-the-meaning-of-this-splint-warning-and-what-might-i-be-doing-wrong

[claucece]

We decided around the 'manifest arrays' to change them to arrays everywhere and then define in the documentation of which size.

[olabini]

So, most of the above comments are not relevant anymore. In order to work on splint issues, look in .splintrc, comment one of the suppressions and try to fix the issue. For reference, the current temporary suppressions are:

+boolint
+voidabstract
+matchanyintegral
-branchstate
-compdef
-compdestroy
-compmempass
-exportlocal
-fixedformalarray
-globstate
-incondefs
-mayaliasunique
-mustfreefresh
-mustfreeonly
-noeffect
-nullassign
-nullpass
-nullret
-nullstate
-statictrans
-temptrans
-type
-unqualifiedtrans
-unrecog
-usedef
-usereleased