CONTRIBUTING: explain how assertions work for us
This commit is contained in:
54
CONTRIBUTING
54
CONTRIBUTING
@@ -43,5 +43,59 @@ GNU General Public License, version 2 or later, or, if another license
|
|||||||
is specified as governing the file or directory being modified, such
|
is specified as governing the file or directory being modified, such
|
||||||
other license. See the file COPYING in this directory for details.
|
other license. See the file COPYING in this directory for details.
|
||||||
|
|
||||||
|
Assertions in NetworkManager code
|
||||||
|
=================================
|
||||||
|
|
||||||
|
There are different kind of assertions. Use the one that is appropriate.
|
||||||
|
|
||||||
|
1) g_return_*() from glib. This is usually enabled in release builds and
|
||||||
|
can be disabled with G_DISABLE_CHECKS define. This uses g_log() with
|
||||||
|
a cG_LOG_LEVEL_CRITICAL level (which allows the program to continue,
|
||||||
|
until G_DEBUG=fatal-criticals or G_DEBUG=fatal-warnings is set). As such,
|
||||||
|
this is the preferred way for assertions that are commonly enabled.
|
||||||
|
|
||||||
|
Make a mild attempt to work around such assertion failure, but don't try
|
||||||
|
to hard. A failure of g_return_*() assertion might allow the process
|
||||||
|
to continue, but there is no guarantee.
|
||||||
|
|
||||||
|
2) nm_assert() from NetworkManager. This is disabled by default in release
|
||||||
|
builds, but enabled if you build --with-more-assertions. See "WITH_MORE_ASSERTS"
|
||||||
|
define. This is preferred for assertions that are expensive to check or
|
||||||
|
nor necessary to check frequently (stuff that is really not expected to
|
||||||
|
fail). Use this deliberately and assume it's not present in production builds.
|
||||||
|
|
||||||
|
3) g_assert() from glib. This is used in unit tests and commonly enabled
|
||||||
|
in release builds. It can be disabled with G_DISABLE_ASSERT assert
|
||||||
|
define. Since this results in a hard crash on assertion failure, you
|
||||||
|
should almost always prefer g_return_*() over this (except unit tests).
|
||||||
|
|
||||||
|
4) assert() from <assert.h>. It is usually enabled in release builds and
|
||||||
|
can be disabled with NDEBUG define. Don't use it in NetworkManager,
|
||||||
|
it's basically like g_assert().
|
||||||
|
|
||||||
|
5) g_log() from glib. These are always compiled in, depending on the levels
|
||||||
|
these are assertions too. G_LOG_LEVEL_ERROR aborts the program, G_LOG_LEVEL_CRITICAL
|
||||||
|
logs a critical warning (like g_return_*(), see G_DEBUG=fatal-criticals)
|
||||||
|
and G_LOG_LEVEL_WARNING logs a warning (see G_DEBUG=fatal-warnings).
|
||||||
|
G_LOG_LEVEL_DEBUG level is usually not printed, unless G_MESSAGES_DEBUG environment
|
||||||
|
is set.
|
||||||
|
In general, avoid using g_log() in NetworkManager. We have nm-logging instead.
|
||||||
|
From a library like libnm it might make sense to log warnings (if someting
|
||||||
|
is really wrong) or debug messages. But better don't. If it's important,
|
||||||
|
find a way to report the notification via the API to the caller. If it's
|
||||||
|
not important, keep silent.
|
||||||
|
|
||||||
|
6) g_warn_if_*() from glib. These are always compiled in and log a G_LOG_LEVEL_WARNING
|
||||||
|
warning. Don't use this.
|
||||||
|
|
||||||
|
7) G_TYPE_CHECK_INSTANCE_CAST() from glib. Unless building with "WITH_MORE_ASSERTS",
|
||||||
|
we disable G_DISABLE_CAST_CHECKS. This means, cast macros like NM_DEVICE(ptr)
|
||||||
|
translate to plain C pointer casts. Use the cast macros deliberately, in production
|
||||||
|
code they are cheap, with debugging enabled they assert that the pointer is valid.
|
||||||
|
|
||||||
|
Of course, every assertion failure is a bug, and they must not have side effects.
|
||||||
|
Theoretically, you are welcome to disable G_DISABLE_CHECKS and G_DISABLE_ASSERT
|
||||||
|
in production builds. In practice, nobody tests such a configuration, so beware.
|
||||||
|
|
||||||
|
For testing, you also want to run NetworkManager with G_DEBUG=fatal-warnings
|
||||||
|
to crash un critical and warn g_log() messages.
|
||||||
|
Reference in New Issue
Block a user