Project

General

Profile

Coding standards » History » Version 36

osmith, 01/27/2021 05:13 PM
we decided on gnu11 in last team meeting, document it here. related: https://gerrit.osmocom.org/q/topic:cstd

1 1 k
h1. Osmocom coding standards
2 34 fixeria
3
{{>toc}}
4 1 k
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 27 blobb
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). 
9 8 laforge
10 27 blobb
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 36 osmith
The dialect of C is gnu11, and we enforce this in CFLAGS.
13
14 31 neels
See also our [[Guidelines for API documentation]].
15
16 10 laforge
h2. Development Process
17
18
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.
19
20 28 blobb
For further information on the Linux kernel development process, please see https://www.kernel.org/doc/html/latest/process/development-process.html
21 10 laforge
22
The most important parts are summarized below:
23
24
h2. The lifecycle of a patch
25
26 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.
27 10 laforge
28
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
29
treatment will come in later sections.
30
31
The stages that a patch goes through are, generally:
32
33
* *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.
34
35 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.
36 10 laforge
37 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.
38 10 laforge
39
* *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.
40
41
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.
42
43
44
h3. The importance of submitting your changes to mainline
45
46
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
47
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.
48
49
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:
50
51
* 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.
52
53
* 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.
54
 
55
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.
56
57
* 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.
58
59
* 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.
60
61
* 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.
62
63
* 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.
64
65
* 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.
66
67
h2. Licensing
68
69 29 blobb
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.
70 1 k
71 10 laforge
h2. Submitting Patches
72 13 laforge
73 15 neels
Before you send a patch, please read all of the guidelines below.
74 1 k
75 35 Hoernchen
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.**
76 15 neels
77 35 Hoernchen
**For all other projects, we accept patch submissions sent to the appropriate "mailing list":https://lists.osmocom.org/mailman/listinfo.**
78 15 neels
79
Note: Osmocom projects are mirrored on github, but all and any pull requests will be rejected by a bot.
80 1 k
81 30 blobb
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.
82 12 neels
83 8 laforge
The generally important topics are:
84
85
h3. Submit patches against a current source tree
86
87
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.
88
89
h3. Use unified diff format
90
91 20 neels
In case of gerrit submissions, using 'git push' or 'git-review' will ensure the correct format.
92
93
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).
94 8 laforge
95
h3. Have meaningful descriptions in your patches
96
97 21 neels
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.
98
99
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.
100
101 33 neels
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>
102 21 neels
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>
103
104
Do not re-state the obvious, but explain the context:
105
106 22 neels
* 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>
107 33 neels
* 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>
108 21 neels
* If fixing an issue reported by coverity scan, include the CID, e.g. include a line: <pre>Fixes: coverity scan CID#99999</pre>
109
* If fixing an error, include a 1:1 paste of the error message fixed and/or explain the scenario.
110
* If adding a feature, explain the use case.
111
112
For commit log examples, see https://cgit.osmocom.org/libosmocore/log/
113 8 laforge
114
h3. Separate your changes
115
116 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)
117 1 k
118 11 zecke
h3. Have small methods and allow for unit testing
119
120
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...
121
122
h3. Update test results
123
124
If your code is changing test results, have a look at the change and if necessary update it.
125
126 8 laforge
h3. Respond to review comments
127 11 zecke
128 25 neels
h3. On mailing lists, include PATCH in the email subject
129 8 laforge
130
h2. Rules of thumb
131
132
Some general rules of thumb:
133
* Consider using tools like @Lindent@ and @checkpatch.pl@ or @universalindentgui@ before submitting code
134
* Run @make@, @make check@ and @make distcheck@ on each of your commits to ensure the build
135 23 neels
* 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)
136 17 laforge
137
h2. Naming conventions
138
139
For new code, we generally prefer the following naming conventions:
140
141
* symbols (functions, global static data)
142
** always use the *osmo_* prefix for symbols exported by libraries
143 24 neels
** 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)
144 17 laforge
* libraries
145
** use *libosmo-* prefix for library name
146
* executables
147
** use *osmo-* name prefix for executable programs
Add picture from clipboard (Maximum size: 48.8 MB)