New upstream version 1.19.90 upstream/1.19.90

1 files changed, 94 insertions, 21 deletions

diff --git a/CONTRIBUTING b/CONTRIBUTING
index 67694bd8..febfdf04 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -1,19 +1,17 @@
-Guidelines for Contributing:
-
-1) Platform-specific functionality (for example, location of binaries that
-NetworkManager calls, or functionality used only on some platforms or
-distribution, like resolvconf) should be configurable at build time, with the
-normal autoconf mechanisms for putting a #define in config.h (AC_DEFINE), then
-with #ifdef MY_DEFINE / #endif in the code.
-
-2) Coding standards are generally GNOME coding standards, with these exceptions:
-	a) 4 space tabs  (_not_ 8-space tabs)
-	b) REAL tabs (_not_ a mix of tabs and spaces in the initial indent)
-	c) spaces used to align continuation lines past the indent point of the
-	   first statement line, like so:
-
-		if (some_really_really_long_variable_name &&
-		    another_really_really_long_variable_name) {
+Guidelines for Contributing
+===========================
+
+Coding Standard
+---------------
+
+Coding standards are generally GNOME coding standards, with these exceptions:
+    a) 4 space tabs  (_not_ 8-space tabs)
+    b) REAL tabs (_not_ a mix of tabs and spaces in the initial indent)
+    c) spaces used to align continuation lines past the indent point of the
+       first statement line, like so:
+
+		if (   some_really_really_long_variable_name
+		    && another_really_really_long_variable_name) {
 			...
 		}
 
@@ -36,12 +34,87 @@ with #ifdef MY_DEFINE / #endif in the code.
     GOOD: #define MY_CONSTANT 42
     BAD:  static const unsigned myConstant = 42;
 
-3) Legal:
+Legal
+-----
+
+NetworkManager is partly licensed under terms of GNU Lesser General Public License
+version 2 or later (LGPL-2.0+). That is for example the case for libnm.
+For historical reasons, the daemon itself is licensed under terms of GNU General
+Public License, version 2 or later (GPL-2.0+). See the license comment in the source
+files.
+Note that all new contributions to NetworkManager MUST be made under terms of
+LGPL-2.0+, that is also the case for parts that are currently licensed GPL-2.0+.
+The reason for that is that we might eventually relicense everything as LGPL and
+new contributions already must agree with that future change.
+
+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
+  G_LOG_LEVEL_CRITICAL level (which allows the program to continue,
+  unless G_DEBUG=fatal-criticals or G_DEBUG=fatal-warnings is set). As such,
+  this is usually the preferred way for assertions that are supposed to be
+  enabled by default.
+
+  Optimally, after a g_return_*() failure the program can still continue. This is
+  also the reason why g_return_*() is preferable over g_assert().
+  For example, that is often not given for functions that return a GError, because
+  g_return_*() will return failure without setting the error output. That often leads
+  to a crash immidiately after, because the caller requires the GError to be set.
+  Make a reasonable effort so that an assertion failure may allow the process
+  to proceed. But don't put too much effort in it. After all, it's an assertion
+  failure that is not supposed to happen either way.
+
+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. It's also for conditions that can easily
+  verified to be true and where future refactoring is unlikley to break that
+  condition.
+  Use this deliberately and assume it is removed from 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 in 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 logging level
+  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
+  which logs to syslog/systemd-journald.
+  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.
+  In particular, don't use levels G_LOG_LEVEL_CRITICAL and G_LOG_LEVEL_WARNING because
+  these are effectively assertions and we want to run with G_DEBUG=fatal-warnings.
+
+6) g_warn_if_*() from glib. These are always compiled in and log a G_LOG_LEVEL_WARNING
+  warning. Don't use this.
 
-All original contributions to NetworkManager are licensed under the
-GNU General Public License, version 2 or later, or, if another license
-is specified as governing the file or directory being modified, such
-other license. See the file COPYING in this directory for details.
+7) G_TYPE_CHECK_INSTANCE_CAST() from glib. Unless building with "WITH_MORE_ASSERTS",
+  we set G_DISABLE_CAST_CHECKS. This means, cast macros like NM_DEVICE(ptr)
+  translate to plain C pointer casts. Use such cast macros deliberately, in production
+  code they are cheap, with more asserts enabled the check that the pointer type is
+  suitable.
 
+Of course, every assertion failure is a bug, and calling it must have no 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 environment variable
+G_DEBUG=fatal-warnings to crash upon G_LOG_LEVEL_CRITICAL and G_LOG_LEVEL_WARNING
+g_log() message. NetworkManager won't use these levels for regular logging
+but for assertions.