[Swan-dev] Fwd: New Defects reported by Coverity Scan for antonyantony/libreswan

[Andrew Cagney]

https://github.com/libreswan/libreswan/commit/bf1b1af577d70a800436b417b94cefba4eb394b2

This comment was removed:

- * Note that, for pexpect(EXPRESSION), the expanded ASSERTION is not
- * wrapped in parenthesis as doing that suppresses the warning -Wparen
- * (i.e., assignment in an expression).

and then the original macro was changed so that EXPRESSION _is_ inside paren:

> | It might be useful to figure out of llvm supports this feature, or just go with:
> |
> | + ((ASSERTION) ? true : (libreswan_pexpect_fail(__func__, \
> | + PASSERT_BASENAME, __LINE__, \
> | + #ASSERTION), false))
> |
> | except that wraps ASSERTION in paren :-(

This means, for any compiler other than GCC, -Wparen is going to be
suppressed.  LLVM for instance.
But this all begs the question, why is a special GCC code path needed?

>
> | (does libreswan_pexpect.c which contained libreswan_pexpect() need a rename?)
>
> The general rule at the begining of the code was: one .c for each .h
> and one .h for each .c.  Exceptions needed justification.

I suspect you'd object, and much of the klips code would break, if I
tried to move most the library code into libreswan.c - after all
libreswan.h is where all those declarations live! :-)

For programs, having xxx.c and xxx.h is a nice consistency, and one
I've tended to follow.

However, for libraries, one of the expectations is that a reference to
one function doesn't suck in the kitchen sink, so both one function
per-file and/or closely related functions per-file are pretty
standard..

> This was documented but nobody maintains or reads documentation.
>
> Since pexpect and libreswan_pexpect_fail are declared in lswlog.h,
> libreswan_pexpect_fail should be defined in lswlog.c.