Project

General

Profile

Coding standards » History » Version 20

neels, 03/18/2017 10:59 AM

1 9 laforge
{{>toc}}
2
3 8 laforge
h1. Osmocom coding standards
4 1
5 10 laforge
h2. Coding Style
6 8 laforge
7
In general, for the C-language projects we maintain in the Osmocom project, we follow the coding style
8 13 laforge
of the Linux kernel. See https://www.kernel.org/doc/Documentation/CodingStyle for further information.
9 8 laforge
10 1
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.
11 10 laforge
12
h2. Development Process
13
14
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.
15
16
For further information on the Linux kernel development process, please see https://www.kernel.org/doc/Documentation/development-process/
17
18
The most important parts are summarized below:
19
20
h2. The lifecycle of a patch
21
22 13 laforge
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.
23 10 laforge
24
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
25
treatment will come in later sections.
26
27
The stages that a patch goes through are, generally:
28
29
* *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.
30
31 19 neels
* *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.
32 10 laforge
33 19 neels
* *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.
34 10 laforge
35
* *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.
36
37
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.
38
39
40
h3. The importance of submitting your changes to mainline
41
42
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
43
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.
44
45
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:
46
47
* 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.
48
49
* 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.
50
 
51
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.
52
53
* 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.
54
55
* 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.
56
57
* 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.
58
59
* 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.
60
61
* 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.
62
63
h2. Licensing
64
65
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
66 1
which are not covered by a compatible license cannot be accepted.
67
68 10 laforge
h2. Submitting Patches
69 13 laforge
70 15 neels
Before you send a patch, please read all of the guidelines below.
71 1
72 16 neels
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.
73 15 neels
74
For all other projects, we accept patch submissions sent to the appropriate "mailing list":https://lists.osmocom.org/mailman/listinfo.
75
76
Note: Osmocom projects are mirrored on github, but all and any pull requests will be rejected by a bot.
77 1
78 12 neels
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.
79
80 8 laforge
The generally important topics are:
81
82
h3. Submit patches against a current source tree
83
84
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.
85
86
h3. Use unified diff format
87
88 20 neels
In case of gerrit submissions, using 'git push' or 'git-review' will ensure the correct format.
89
90
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).
91 8 laforge
92
h3. Have meaningful descriptions in your patches
93
94
Every commit/patch should have an explanation in the commit message.
95
96
h3. Separate your changes
97
98 11 zecke
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)
99 1
100 11 zecke
h3. Have small methods and allow for unit testing
101
102
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...
103
104
h3. Update test results
105
106
If your code is changing test results, have a look at the change and if necessary update it.
107
108 8 laforge
h3. Respond to review comments
109 11 zecke
110 8 laforge
h3. Include PATCH in the subject
111
112
h2. Rules of thumb
113
114
Some general rules of thumb:
115
* Consider using tools like @Lindent@ and @checkpatch.pl@ or @universalindentgui@ before submitting code
116
* Run @make@, @make check@ and @make distcheck@ on each of your commits to ensure the build
117
* Use @git send-email@ or @git imap-send@ to send your patches (otherwise your mail client may wrap around lines and corrupt your patch)
118 17 laforge
119
h2. Naming conventions
120
121
For new code, we generally prefer the following naming conventions:
122
123
* symbols (functions, global static data)
124
** always use the *osmo_* prefix for symbols exported by libraries
125
** never use the *osmo_* prefix for symbols inside applications
126
* libraries
127
** use *libosmo-* prefix for library name
128
* executables
129
** use *osmo-* name prefix for executable programs
Add picture from clipboard (Maximum size: 48.8 MB)