sparse simply doesn't like our build assertions on packed structures.
It seems that its ideas about struct packing are different from GCC's:
../lib/cfm.h:50:1: error: invalid bitfield width, -1.
../lib/packets.h:206:1: error: invalid bitfield width, -1.
../lib/packets.h:213:1: error: invalid bitfield width, -1.
../lib/packets.h:367:1: error: invalid bitfield width, -1.
sparse isn't generating code so we don't really care how it lays out
structures. We might as well just skip the assertions, as done here.
Until now, when the poll_loop module's log level was turned up to "debug",
it would log a backtrace of the call stack for the event that caused poll()
to wake up in poll_block(). This was pretty useful from time to time to
find out why ovs-vswitchd was using more CPU than expected, because we
could find out what was causing it to wake up.
But there were some issues. One is simply that the backtrace was printed
as a series of hexadecimal numbers, so GDB or another debugger was needed
to translate it into human-readable format. Compiler optimizations meant
that even the human-readable backtrace wasn't, in my experience, as helpful
as it could have been. And, of course, one needed to have the binary to
interpret the backtrace. When the backtrace couldn't be interpreted or
wasn't meaningful, there was essentially nothing to fall back on.
This commit changes the way that "debug" logging for poll_block() wakeups
works. Instead of logging a backtrace, it logs the source code file name
and line number of the call to a poll_loop function, using __FILE__ and
__LINE__. This is by itself much more meaningful than a sequence of
hexadecimal numbers, since no additional interpretation is necessary. It
can be useful even if the Open vSwitch version is only approximately known.
In addition to the file and line, this commit adds, for wakeups caused by
file descriptors, information about the file descriptor itself: what kind
of file it is (regular file, directory, socket, etc.), the name of the file
(on Linux only), and the local and remote endpoints for socket file
descriptors.
Here are a few examples of the new output format:
932-ms timeout at ../ofproto/in-band.c:507
[POLLIN] on fd 20 (192.168.0.20:35388<->192.168.0.3:6633) at ../lib/stream-fd.c:149
[POLLIN] on fd 7 (FIFO pipe:[48049]) at ../lib/fatal-signal.c:168
exit(EXIT_FAILURE) will make a monitoring process (the one created by
--monitor) think that it should exit. But the most likely reason for
out_of_memory() to be called is a bug: probably, the process is trying
to allocate more memory than there is available address space, e.g.
something like malloc(-1). So it's better, in my opinion, to call abort()
instead, so that the monitor process restarts the daemon and we are more
likely to stay alive and, in addition, get a core dump and a useful bug
report.
I decided to implement a new general-purpose function for this purpose in
case we run into other similar situations in the future.
(I haven't actually run into this problem in practice. This commit is
just speculation about what is better behavior.)
Static analyzers hate strncpy(). This new function shares its property of
initializing an entire buffer, without its nasty habit of failing to
null-terminate long strings.
Coverity #10697,10696,10695,10694,10693,10692,10691,10690.
Many OVS functions return 0, EOF, or errno. There are several places in the
codebase where a return value is converted to a string. All must decide whether
the return value is set, and if it is, whether it is an errno value, EOF, or
otherwise invalid. This commit consolidates that code.
Reviewed by Ben Pfaff.
This macro is a variant on CONTAINER_OF that takes an object pointer
instead of a type name as its second argument. In the following commit
this will simplify many users of CONTAINER_OF.
On sparc, GCC would issue the following warning for every use of
CONTAINER_OF:
warning: cast increases required alignment of target type
This is a false positive: assuming that the data structure that it is
applied to was allocated properly, the use of CONTAINER_OF to reach it is
valid. And if it was not allocated properly, then code that accesses it
other ways will have trouble too.
Some systems complain when certain functions' return values are not
checked. This commit fixes those warnings.
Creating ignore() function suggested by Ben Pfaff.
ROUND_UP rounds up to a multiple of a given value. That means that
bitmap_allocate() was allocating one byte for each bit in the bitmap,
which is clearly excessive.
Instead, just allocate one bit for every bit in the bitmap.
