Blame CONTRIBUTING

Packit Service b23acc
Guidelines for Contributing
Packit Service b23acc
===========================
Packit Service b23acc
Packit Service b23acc
Coding Standard
Packit Service b23acc
---------------
Packit Service b23acc
Packit Service b23acc
Coding standards are generally GNOME coding standards, with these exceptions:
Packit Service b23acc
    a) 4 space tabs  (_not_ 8-space tabs)
Packit Service b23acc
    b) REAL tabs (_not_ a mix of tabs and spaces in the initial indent)
Packit Service b23acc
    c) spaces used to align continuation lines past the indent point of the
Packit Service b23acc
       first statement line, like so:
Packit Service b23acc
Packit Service b23acc
		if (   some_really_really_long_variable_name
Packit Service b23acc
		    && another_really_really_long_variable_name) {
Packit Service b23acc
			...
Packit Service b23acc
		}
Packit Service b23acc
Packit Service b23acc
* Keep a space between the function name and the opening '('.
Packit Service b23acc
    GOOD: g_strdup (x)
Packit Service b23acc
    BAD:  g_strdup(x)
Packit Service b23acc
Packit Service b23acc
* C-style comments
Packit Service b23acc
    GOOD: f(x);  /* comment */
Packit Service b23acc
    BAD:  f(x);  // comment
Packit Service b23acc
Packit Service b23acc
* Keep assignments in the variable declaration area pretty short.
Packit Service b23acc
    GOOD: MyObject *object;
Packit Service b23acc
    BAD: MyObject *object = complex_and_long_init_function(arg1, arg2, arg3);
Packit Service b23acc
Packit Service b23acc
* 80-cols is a guideline, don't make the code uncomfortable in order to fit in
Packit Service b23acc
  less than 80 cols.
Packit Service b23acc
Packit Service b23acc
* Constants are CAPS_WITH_UNDERSCORES and use the preprocessor.
Packit Service b23acc
    GOOD: #define MY_CONSTANT 42
Packit Service b23acc
    BAD:  static const unsigned myConstant = 42;
Packit Service b23acc
Packit Service b23acc
Legal
Packit Service b23acc
-----
Packit Service b23acc
Packit Service b23acc
NetworkManager is partly licensed under terms of GNU Lesser General Public License
Packit Service b23acc
version 2 or later (LGPL-2.1+). That is for example the case for libnm.
Packit Service b23acc
For historical reasons, the daemon itself is licensed under terms of GNU General
Packit Service b23acc
Public License, version 2 or later (GPL-2.0+). See the license comment in the source
Packit Service b23acc
files.
Packit Service b23acc
Note that all new contributions to NetworkManager MUST be made under terms of
Packit Service b23acc
LGPL-2.1+, that is also the case for parts that are currently licensed GPL-2.0+.
Packit Service b23acc
The reason for that is that we might eventually relicense everything as LGPL and
Packit Service b23acc
new contributions already must agree with that future change.
Packit Service b23acc
Packit Service b23acc
Assertions in NetworkManager code
Packit Service b23acc
---------------------------------
Packit Service b23acc
Packit Service b23acc
There are different kind of assertions. Use the one that is appropriate.
Packit Service b23acc
Packit Service b23acc
1) g_return_*() from glib. This is usually enabled in release builds and
Packit Service b23acc
  can be disabled with G_DISABLE_CHECKS define. This uses g_log() with
Packit Service b23acc
  G_LOG_LEVEL_CRITICAL level (which allows the program to continue,
Packit Service b23acc
  unless G_DEBUG=fatal-criticals or G_DEBUG=fatal-warnings is set). As such,
Packit Service b23acc
  this is usually the preferred way for assertions that are supposed to be
Packit Service b23acc
  enabled by default.
Packit Service b23acc
Packit Service b23acc
  Optimally, after a g_return_*() failure the program can still continue. This is
Packit Service b23acc
  also the reason why g_return_*() is preferable over g_assert().
Packit Service b23acc
  For example, that is often not given for functions that return a GError, because
Packit Service b23acc
  g_return_*() will return failure without setting the error output. That often leads
Packit Service b23acc
  to a crash immidiately after, because the caller requires the GError to be set.
Packit Service b23acc
  Make a reasonable effort so that an assertion failure may allow the process
Packit Service b23acc
  to proceed. But don't put too much effort in it. After all, it's an assertion
Packit Service b23acc
  failure that is not supposed to happen either way.
Packit Service b23acc
Packit Service b23acc
2) nm_assert() from NetworkManager. This is disabled by default in release
Packit Service b23acc
  builds, but enabled if you build --with-more-assertions. See "WITH_MORE_ASSERTS"
Packit Service b23acc
  define. This is preferred for assertions that are expensive to check or
Packit Service b23acc
  nor necessary to check frequently. It's also for conditions that can easily
Packit Service b23acc
  verified to be true and where future refactoring is unlikley to break that
Packit Service b23acc
  condition.
Packit Service b23acc
  Use this deliberately and assume it is removed from production builds.
Packit Service b23acc
Packit Service b23acc
3) g_assert() from glib. This is used in unit tests and commonly enabled
Packit Service b23acc
  in release builds. It can be disabled with G_DISABLE_ASSERT assert
Packit Service b23acc
  define. Since this results in a hard crash on assertion failure, you
Packit Service b23acc
  should almost always prefer g_return_*() over this (except in unit tests).
Packit Service b23acc
Packit Service b23acc
4) assert() from <assert.h>. It is usually enabled in release builds and
Packit Service b23acc
  can be disabled with NDEBUG define. Don't use it in NetworkManager,
Packit Service b23acc
  it's basically like g_assert().
Packit Service b23acc
Packit Service b23acc
5) g_log() from glib. These are always compiled in, depending on the logging level
Packit Service b23acc
  these are assertions too. G_LOG_LEVEL_ERROR aborts the program, G_LOG_LEVEL_CRITICAL
Packit Service b23acc
  logs a critical warning (like g_return_*(), see G_DEBUG=fatal-criticals)
Packit Service b23acc
  and G_LOG_LEVEL_WARNING logs a warning (see G_DEBUG=fatal-warnings).
Packit Service b23acc
  G_LOG_LEVEL_DEBUG level is usually not printed, unless G_MESSAGES_DEBUG environment
Packit Service b23acc
  is set.
Packit Service b23acc
  In general, avoid using g_log() in NetworkManager. We have nm-logging instead
Packit Service b23acc
  which logs to syslog/systemd-journald.
Packit Service b23acc
  From a library like libnm it might make sense to log warnings (if someting
Packit Service b23acc
  is really wrong) or debug messages. But better don't. If it's important,
Packit Service b23acc
  find a way to report the notification via the API to the caller. If it's
Packit Service b23acc
  not important, keep silent.
Packit Service b23acc
  In particular, don't use levels G_LOG_LEVEL_CRITICAL and G_LOG_LEVEL_WARNING because
Packit Service b23acc
  these are effectively assertions and we want to run with G_DEBUG=fatal-warnings.
Packit Service b23acc
Packit Service b23acc
6) g_warn_if_*() from glib. These are always compiled in and log a G_LOG_LEVEL_WARNING
Packit Service b23acc
  warning. Don't use this.
Packit Service b23acc
Packit Service b23acc
7) G_TYPE_CHECK_INSTANCE_CAST() from glib. Unless building with "WITH_MORE_ASSERTS",
Packit Service b23acc
  we set G_DISABLE_CAST_CHECKS. This means, cast macros like NM_DEVICE(ptr)
Packit Service b23acc
  translate to plain C pointer casts. Use such cast macros deliberately, in production
Packit Service b23acc
  code they are cheap, with more asserts enabled the check that the pointer type is
Packit Service b23acc
  suitable.
Packit Service b23acc
Packit Service b23acc
Of course, every assertion failure is a bug, and calling it must have no side effects.
Packit Service b23acc
Packit Service b23acc
Theoretically, you are welcome to disable G_DISABLE_CHECKS and G_DISABLE_ASSERT
Packit Service b23acc
in production builds. In practice, nobody tests such a configuration, so beware.
Packit Service b23acc
Packit Service b23acc
For testing, you also want to run NetworkManager with environment variable
Packit Service b23acc
G_DEBUG=fatal-warnings to crash upon G_LOG_LEVEL_CRITICAL and G_LOG_LEVEL_WARNING
Packit Service b23acc
g_log() message. NetworkManager won't use these levels for regular logging
Packit Service b23acc
but for assertions.