Project

General

Profile

Actions

Coding standards » History » Revision 14

« Previous | Revision 14/37 (diff) | Next »
neels, 11/15/2016 02:20 PM


Osmocom coding standards

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/Documentation/CodingStyle for further information.

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.

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/Documentation/development-process/

The most important parts are summarized below:

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 the relevant mailing list, and developers on that list reply with any comments they may have. 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 managed by Linus Torvalds. 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.

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.

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.

Submitting Patches

The detailed process differs a bit from Osmocom sub-project to sub-project. The cellular infrastructure projects (e.g. OpenBSC) and their associated libraries have switched to a Gerrit based submission + review process, see Gerrit for more details. If you don't want to use Gerrit, you can use the e-mail based submission process below, but beware it will create extra work for the maintainers, so please avoid it when possible.

In general, we accept patch submissions sent to the appropriate mailing list. Before you send a patch, please read all of the guidelines below.

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

The generally important topics are:

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.

Use unified diff format

It's best to let git generate the patches, so you don't have to worry about that and preferable you use git send-email to send them.

Have meaningful descriptions in your patches

Every commit/patch should have an explanation in the commit message.

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)

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

Update test results

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

Respond to review comments

Include PATCH in the subject

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
  • Use git send-email or git imap-send to send your patches (otherwise your mail client may wrap around lines and corrupt your patch)

Updated by neels almost 6 years ago · 14 revisions

Add picture from clipboard (Maximum size: 48.8 MB)