netlink: change snl(3) API for more robust handling of malloc errors

[glebius]

Summary

Before this change the API was kind of upside down: a function that
allocates memory was returning void, while a function that doesn't
allocate memory neither makes any system calls could fail. The former is
snl_init_writer() and the latter is snl_finalize_msg().

Now result of snl_init_writer() should be checked and snl_finalize_msg()
always succeeds. We don't store the error for the later. This makes
snl_abort_msg() unused inside the library, but we would leave it there,
since it may be used by an application to reset state of the writer.
While here, also assert(3) that the library use is correct, rather than
try to handle the misuse. Certain functions require writer to be an
initialized state, and certain require the opposite.

Note that this change leaves world compilable, as well as utilities
working, but libpftctl now has even more unchecked calls into the snl(3)
library. This will be handled separately.

[glebius]

Note for @kp . Even without this change there is a lot of bad copy & paste in libpfctl and lack of result checking. The change is not expected to break anything beyond current level of brokenness. Once we accept this revision, and maybe a couple future ones, I promise to do a sweep over libpfctl. It is too early to do it now, since I might find more potential malloc failures not propagated to API user as well as general API pitfalls.

[kp]

In D48477#1106107, @glebius wrote:

Note for @kp . Even without this change there is a lot of bad copy & paste in libpfctl and lack of result checking. The change is not expected to break anything beyond current level of brokenness. Once we accept this revision, and maybe a couple future ones, I promise to do a sweep over libpfctl. It is too early to do it now, since I might find more potential malloc failures not propagated to API user as well as general API pitfalls.

To be honest, I usually write code on the assumption that memory allocation in userspace never fails. We overallocate by default, so that's the (by far) most common case. Failure code basically never gets tested, and can fairly safely be assumed to be broken.

I'm not going to stop anyone from trying to make it correct, but I'm just not very concerned about it.

sys/netlink/netlink_snl.h

1051

return (false); ?