Cellular Network Infrastructure

h1. Osmocom coding standards

{{>toc}}

h2. Coding Style

In general, for the C-language projects we maintain in the Osmocom project, we follow the coding style

of the Linux kernel. See https://www.kernel.org/doc/html/latest/process/coding-style.html for further information. The sole exception to this is that we accept line lengths of up to 120 characters (Linux kernel requires 80 character limit). 

We will not debate about the coding style and whether some other style is better or worse. The important factor is not so much _which_ coding style is followed, as long as there is one well-defined coding style and that all developers adhere to it.

The dialect of C is gnu11, and we enforce this in CFLAGS.

See also our [[Guidelines for API documentation]].

h2. Development Process

Once again, the Osmocom project follows processes defined for the Linux kernel development whenever applicable.  Of course, as Osmocom is  much smaller project, some simplifications apply.

For further information on the Linux kernel development process, please see https://www.kernel.org/doc/html/latest/process/development-process.html

The most important parts are summarized below:

h2. The lifecycle of a patch

Patches do not go directly from the developer's keyboard into the mainline project.  There is, instead, a somewhat involved (if somewhat informal) process designed to ensure that each patch is reviewed for quality and that each patch implements a change which is desirable to have in the mainline. This process can happen quickly for minor fixes, or, in the case of large and controversial changes, go on for years.  Much developer frustration comes from a lack of understanding of this process or from attempts to circumvent it.

In the hopes of reducing that frustration, this document will describe how a patch gets into the kernel.  What follows below is an introduction which describes the process in a somewhat idealized way.  A much more detailed

treatment will come in later sections.

The stages that a patch goes through are, generally:

* *Design*. This is where the real requirements for the patch - and the way those requirements will be met - are laid out.  Design work is often done without involving the community, but it is better to do this work in the open if at all possible; it can save a lot of time redesigning things later.

* *Review*.  Patches are posted to gerrit, and developers with any comments they may have; for projects not on gerrit.osmocom.org, patches and comments are posted to the appropriate mailing list.  This process should turn up any major problems with a patch if all goes well. Please note that most maintainers also have day jobs, so merging your patch may not be their highest priority.  If your patch is getting feedback about changes that are needed, you should either make those changes or justify why they should not be made.  If your patch has no review complaints but is not being merged by its appropriate maintainer, you should be persistent in updating the patch to the current master versoin so that it applies cleanly and keep sending it for review and merging.

* *Merging into the mainline*.  Eventually, a successful patch will be merged into the mainline repository.  See [[Gerrit#Merge-patch-to-master|Gerrit - Merge patch to master]] for details on review and merging on our gerrit.  More comments and/or problems may surface at this time; it is important that the developer be responsive to these and fix any issues which arise.

* *Long-term maintenance*.  While it is certainly possible for a developer to forget about code after merging it, that sort of behavior tends to leave a poor impression in the development community.  Merging code eliminates some of the maintenance burden, in that others will fix problems caused by API changes.  But the original developer should continue to take responsibility for the code if it is to remain useful in the longer term.

One of the largest mistakes made by developers (or their employers) is to try to cut the process down to a single *merging into the mainline* step.  This approach invariably leads to frustration for everybody involved.

h3. The importance of submitting your changes to mainline

Some companies and developers occasionally wonder why they should bother learning how to work with the Osmocom community and get their code into mainline (the "mainline" being the software maintained by the Osmocom team at git.osmocom.org).  In the short term, contributing code can look like an avoidable expense; it seems easier to

just keep the code separate and support users directly.  The truth of the matter is that keeping code separate ("out of tree") is a false economy.

As a way of illustrating the costs of out-of-tree code, here are a few relevant aspects of the kernel development process; most of these will be discussed in greater detail later in this document.  Consider:

* Code which has been merged into the mainline kernel is available to all users.  There is no need for vendor-specific drivers/builds, downloads, or the hassles of supporting multiple versions of multiple distributions; it all just works, for the developer and for the user.  Incorporation into the mainline solves a large number of distribution and support problems.

* The internal API of the projects is in constant flux.  The lack of a stable internal interface is a deliberate design decision; it allows fundamental improvements to be made at any time and results in higher-quality code. But one result of that policy is that any out-of-tree code requires constant upkeep if it is to work with new kernels.

 

Maintaining out-of-tree code requires significant amounts of work just to keep that code working. Code which is in the mainline, instead, does not require this work as the result of a simple rule requiring any developer who makes an API change to also fix any code that breaks as the result of that change.  So code which has been merged into the mainline has significantly lower maintenance costs.

* Beyond that, code which is in mainline will often be improved by other developers.  Surprising results can come from empowering your user community and customers to improve your product.

* Code is subjected to review, both before and after merging into the mainline.  No matter how strong the original developer's skills are, this review process invariably finds ways in which the code can be improved.  Often review finds severe bugs and security problems.  This is especially true for code which has been developed in a closed environment; such code benefits strongly from review by outside developers.  Out-of-tree code is lower-quality code.

* Participation in the development process is your way to influence the direction of the projects development.  Users who complain from the sidelines are heard, but active developers have a stronger voice - and the ability to implement changes which make the kernel work better for their needs.

* When code is maintained separately, the possibility that a third party will contribute a different implementation of a similar feature always exists.  Should that happen, getting your code merged will become much harder - to the point of impossibility.  Then you will be faced with the unpleasant alternatives of either (1) maintaining a nonstandard features out of tree indefinitely, or (2) abandoning your code and migrating your users over to the in-tree version.

* Contribution of code is the fundamental action which makes the whole process work.  By contributing your code you can add new functionality to the project and provide capabilities and examples which are of use to other developers.  If you have developed code for a project (or are thinking about doing so), you clearly have an interest in the continued success of this platform; contributing code is one of the best ways to help ensure that success.

h2. Licensing

Code is contributed to any Osmocom project under a number of licenses, but all code must be compatible with the respective project license.  This typically is the GNU Affero General Public License, Version 3 (AGPLv3) or Version 2 of the GNU General Public License (GPLv2). Any contributions which are not covered by a compatible license cannot be accepted.

h2. Submitting Patches

Before you send a patch, please read all of the guidelines below.

How to submit patches for Osmocom projects depends on the maintainer. **Most actively maintained Osmocom projects ("see this list":https://gerrit.osmocom.org/#/admin/projects/) have switched to a Gerrit based submission + review process, see [[Gerrit]] for more details.**

**For all other projects, we accept patch submissions sent to the appropriate "mailing list":https://lists.osmocom.org/mailman/listinfo.**

Note: Osmocom projects are mirrored on github, but all and any pull requests will be rejected by a bot.

For submitting patches, the best practises established by the Linux kernel development community should be followed, wherever applicable. See https://www.kernel.org/doc/html/latest/process/5.Posting.html for further information.

The generally important topics are:

h3. Submit patches against a current source tree

At time of submission, our patches should be based on current @master@ branch of the respective project, or another (more current) feature branch of that repository.

h3. Use unified diff format

In case of gerrit submissions, using 'git push' or 'git-review' will ensure the correct format.

In case of mailing list submissions, it's best to let git generate the patches: preferably use 'git send-email' (possibly with 'git format-patch' first).

h3. Have meaningful descriptions in your patches

Every commit/patch should have an explanation in the commit message, that explains intent and method of the patch efficiently and in a way that the changes can be easily understood without having to guess.

You should stick to the imperative form -- e.g. "add foo", "fix bar", "drop baz" -- and avoid empty phrasing like "this patch implements". This is shortest to write and, more importantly, most efficient to read for reviewers.

The first line is a summary, separated by a blank line from the rest. It should be kept brief, and *always* name the general area of the code modified: <pre>vty: fix OML link state printing</pre>

If the patch contains no functional changes, prepend 'cosmetic:' to the summary. For example: <pre>cosmetic: foo.c: drop unused bar()</pre> or <pre>cosmetic: baz.c: rename moo() to goo()</pre>

Do not re-state the obvious, but explain the context:

* If the patch is related to a redmine issue on osmocom.org, include an issue tag like OS#1234. *Always* use this form, never omit the 'OS', so that we can machine parse these references. For example: <pre>Related: OS#1234</pre>

* If the patch depends on another patch (e.g. in another repository), include the other patch's gerrit change-id. For example, include a line <pre>Depends: I0fca8e95ed5c2148b1a7440eff3fc9c7583898df (libosmocore)</pre>

* If fixing an issue reported by coverity scan, include the CID, e.g. include a line: <pre>Fixes: coverity scan CID#99999</pre>

* If fixing an error, include a 1:1 paste of the error message fixed and/or explain the scenario.

* If adding a feature, explain the use case.

For commit log examples, see https://cgit.osmocom.org/libosmocore/log/

h3. Separate your changes

Separate each _logical change_ into a separate patch. Do not mix different concerns (e.g. do not add functionality and change unrelated functionality at the same time, not even coding style clean-ups)

h3. Have small methods and allow for unit testing

Large methods are difficult to read (as they don't fit to the screen), large methods have the tendency to do too many things. Try to split them up so that parts can be tested separately. If your code is parsing a message and then doing something with the result in most cases it is better to separate this (fill a struct with the parse result). This allows to test the parser without a big test fixture. Try to do the same for the decision handling...

h3. Update test results

If your code is changing test results, have a look at the change and if necessary update it.

h3. Respond to review comments

h3. On mailing lists, include PATCH in the email subject

h2. Rules of thumb

Some general rules of thumb:

* Consider using tools like @Lindent@ and @checkpatch.pl@ or @universalindentgui@ before submitting code

* Run @make@, @make check@ and @make distcheck@ on each of your commits to ensure the build

* For mailing lists, use @git send-email@ or @git imap-send@ to send your patches (otherwise your mail client may wrap around lines and corrupt your patch)

h2. Naming conventions

For new code, we generally prefer the following naming conventions:

* symbols (functions, global static data)

** always use the *osmo_* prefix for symbols exported by libraries

** never use the *osmo_* prefix for symbols inside applications (which avoids a naming conflict in case we choose to later move code from an application to a library, adding @osmo_@ in the process)

* libraries

** use *libosmo-* prefix for library name

* executables

** use *osmo-* name prefix for executable programs