CONTRIBUTING 5.63 KB
Newer Older
Thomas Haller's avatar
Thomas Haller committed
1 2
Guidelines for Contributing
===========================
3

Thomas Haller's avatar
Thomas Haller committed
4 5
Coding Standard
---------------
6

Thomas Haller's avatar
Thomas Haller committed
7 8 9 10 11
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:
Dan Williams's avatar
Dan Williams committed
12

Thomas Haller's avatar
Thomas Haller committed
13 14
		if (   some_really_really_long_variable_name
		    && another_really_really_long_variable_name) {
15 16
			...
		}
Dan Williams's avatar
Dan Williams committed
17

Adam Langley's avatar
Adam Langley committed
18 19 20 21
* Keep a space between the function name and the opening '('.
    GOOD: g_strdup (x)
    BAD:  g_strdup(x)

22
* C-style comments
Adam Langley's avatar
Adam Langley committed
23 24 25 26 27 28 29 30 31 32 33 34 35 36
    GOOD: f(x);  /* comment */
    BAD:  f(x);  // comment

* Keep assignments in the variable declaration area pretty short.
    GOOD: MyObject *object;
    BAD: MyObject *object = complex_and_long_init_function(arg1, arg2, arg3);

* 80-cols is a guideline, don't make the code uncomfortable in order to fit in
  less than 80 cols.

* Constants are CAPS_WITH_UNDERSCORES and use the preprocessor.
    GOOD: #define MY_CONSTANT 42
    BAD:  static const unsigned myConstant = 42;

Thomas Haller's avatar
Thomas Haller committed
37 38
Legal
-----
39

40
NetworkManager is partly licensed under terms of GNU Lesser General Public License
41
version 2 or later (LGPL-2.1+). That is for example the case for libnm.
42 43 44
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.
45
Note that all new contributions to NetworkManager MUST be made under terms of
46
LGPL-2.1+, that is also the case for parts that are currently licensed GPL-2.0+.
47 48
The reason for that is that we might eventually relicense everything as LGPL and
new contributions already must agree with that future change.
49

50
Assertions in NetworkManager code
Thomas Haller's avatar
Thomas Haller committed
51
---------------------------------
52

53
There are different kind of assertions. Use the one that is appropriate.
54

55 56
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
Thomas Haller's avatar
Thomas Haller committed
57
  G_LOG_LEVEL_CRITICAL level (which allows the program to continue,
58 59 60 61 62 63 64 65 66 67 68 69
  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.
70 71 72 73

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
74 75 76 77
  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.
78 79 80 81

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
82
  should almost always prefer g_return_*() over this (except in unit tests).
83 84 85 86 87

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().

88
5) g_log() from glib. These are always compiled in, depending on the logging level
89 90 91 92 93
  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.
94 95
  In general, avoid using g_log() in NetworkManager. We have nm-logging instead
  which logs to syslog/systemd-journald.
96 97 98 99
  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.
100 101
  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.
102 103 104 105 106

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",
107 108 109 110 111 112
  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.
113 114 115 116

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.

117 118 119 120
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.