Functions that take a GError** MUST fill it in on error. There is no
need to check whether error is NULL if the function it was passed to
had a failing return value.
Likewise, a proper GError must have a non-NULL message, so there's no
need to double-check that either.
Based-on-patch-by: Dan Winship <danw@gnome.org>
There are far too many "flags". Rename the "flags" to "n_ifa_flags"
which reminds to "ifa_flags" in 'struct ifaddrmsg', but with a
distinctive "n_" prefix.
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
"nm-default.h" should only include all the relevant header files based
on NETWORKMANAGER_COMPILATION. It should not contain definitions on
it's own.
Move the definition of "bool" to "nm-macros-internal.h".
Using strcmp() to test for string equality is a well known pattern.
However the inverse logic still is still hard to grasp especially in
more complex expressions.
nm_streq() should is an alternative to use strcmp(). And there is a counterpart
nm_streq0() which is based on g_strcmp0().
Kernel and systemd have also similar streq() macros.
https://mail.gnome.org/archives/networkmanager-list/2016-February/msg00047.html
It is ugly that nmtst_assert_connection_verifies_after_normalization() would
normalize the argument and modify it. An assertion should not have side-effects.
- "gsystem-local-alloc.h" and <gio/gio.h> are already included via
"nm-default.h". No need to include them separately.
- include "nm-macros-internal.h" via "nm-default.h" and drop all
explict includes.
- in the modified files, ensure that we always include "config.h"
and "nm-default.h" first. As second, include the header file
for the current source file (if applicable). Then follow external
includes and finally internal nm includes.
- include nm headers inside source code files with quotes
- internal header files don't need to include default headers.
They can savely assume that "nm-default.h" is already included
and with it glib, nm-glib.h, nm-macros-internal.h, etc.
It's declared as never returning, making do a better at understanding control
flow. Otherwise it makes a poor guess:
In file included from test-crypto.c:37:
../../shared/nm-test-utils.h:1344:3: error: variable 'family' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
g_assert (FALSE);
^~~~~~~~~~~~~~~~
Fail only printed a message to stderr and exited.
Instead, print it with g_error(), which also breaks
in the debugger and produces a core-dump.
Also, it constructed the message based on an unchecked
format string and constructed a format string dynamically.
Just don't do that.
Also add a comment that these macros are discouraged because
they are cumbersome to write (requiring a test-name and a failure
message).
Helper macros to define GObject property enum,
the obj_properties array, and a _notify() function.
We should move away from invoking property changed notifications by
string, as we already do for signals. This macro simplifies the
implementation of establishes a pattern for naming and usage.
We inconsistently use gulong,guint,int types to store signal handler
id, but the type returned by g_signal_connect() is a gulong.
This has no practical consequences because a int/guint is enough to
store the value, however it is better to use a consistent type, also
because nm_clear_g_signal_handler() accepts a pointer to the signal id
and thus it must be always called with the same pointer type.
Up to now, the "include" directory contained (only) header files that were
used project-wide by libs, core, clients, et al.
Since the directory now also contains a non-header file, the "include"
name is misleading. Instead of adding yet another directory that is
project-wide, with non-header-only content, rename the "include"
directory to "shared".
