|
Packit |
8fb591 |
# Contributing to libyang
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
First of all, thanks for thinking about contribution to libyang! Not all of us
|
|
Packit |
8fb591 |
are C guru, but believe that helping us with docs, tests, helping other users to
|
|
Packit |
8fb591 |
solve their issues / answer ther questions or even letting us to know via
|
|
Packit |
8fb591 |
[issue tracker](https://github.com/CESNET/libyang/issues) that something
|
|
Packit |
8fb591 |
can be done in a better or just different way is really appreciated.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
If you are willing to contribute, you will definitely have to build and install
|
|
Packit |
8fb591 |
libyang first. To do it, please check dependencies and follow build and install
|
|
Packit |
8fb591 |
instructions provided in [README](README.md).
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
If you have something what you believe should be part of the libyang repository,
|
|
Packit |
8fb591 |
add it via [Github Pull Request mechanism](https://help.github.com/articles/about-pull-requests/).
|
|
Packit |
8fb591 |
Remember to explain what you wish to add and why. The best approach is to start
|
|
Packit |
8fb591 |
with creating an issue and discuss possible approaches there. Pull requests can be
|
|
Packit |
8fb591 |
then connected with such issues.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
## Branches
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
There are 2 main branches in libyang project. The default branch is named `master`. It is the
|
|
Packit |
8fb591 |
most stable and tested code which you get by default when cloning the Git repository. The
|
|
Packit |
8fb591 |
`devel` branch introduces new features, API changes or even bugfixes in case `master` and
|
|
Packit |
8fb591 |
`devel` differs significantly at that moment and the fix affects the changed code. There are
|
|
Packit |
8fb591 |
some more branches for work-in-progress features and special `coverity` branch for submitting
|
|
Packit |
8fb591 |
code for the [analysis in the Coverity tool](https://scan.coverity.com/projects/5259) usign
|
|
Packit |
8fb591 |
[Travis CI build](https://travis-ci.org/CESNET/libyang/branches).
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
When you create pull request, think carefully about the branch where the patch belongs to.
|
|
Packit |
8fb591 |
In most cases (if not all), it is the `devel` branch.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
## Issue Ticketing
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
All the communication with the developers is done via [issue tracker](https://github.com/CESNET/libyang/issues).
|
|
Packit |
8fb591 |
You can send us an email, but in case you will ask a question we would think that someone else
|
|
Packit |
8fb591 |
could ask in future, our answer will be just **use the issue tracker**. Private emails are not visible
|
|
Packit |
8fb591 |
for others and we don't want to answer the same questions.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
So when you are goingto submit a new issue, **please:**
|
|
Packit |
8fb591 |
* check that the issue you are having is not already solved in the devel branch,
|
|
Packit |
8fb591 |
* go through the present issues (in case of question, it can be already a closed issue) in the tracker,
|
|
Packit |
8fb591 |
* give it as descriptive title as possible,
|
|
Packit |
8fb591 |
* separate topics - solving multiple issues in one ticket hides the issues from others,
|
|
Packit |
8fb591 |
* provide as much relevant information as possible (versions, logs, input data, etc.).
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
## libyang Coding Style
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
When you are going to contribute C code, please follow these coding style guidelines.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
### Basics
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
- Use space instead of tabs for indentations.
|
|
Packit |
8fb591 |
- There is no strict limit for the line length, However, try to keep lines in a
|
|
Packit |
8fb591 |
reasonable length (120 characters).
|
|
Packit |
8fb591 |
- Avoid trailing spaces on lines.
|
|
Packit |
8fb591 |
- Put one blank line between function definitions.
|
|
Packit |
8fb591 |
- Don't mix declarations and code within a block. Similarly, don't use
|
|
Packit |
8fb591 |
declarations in iteration statements.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
### Naming
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Use underscores to separate words in an identifier: `multi_word_name`.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Use lowercase for most names. Use uppercase for macros, macro parameters and
|
|
Packit |
8fb591 |
members of enumerations.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Do not use names that begin with `_`. If you need a name for "internal use
|
|
Packit |
8fb591 |
only", use `__` as a suffix instead of a prefix.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
### Comments
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Avoid `//` comments. Use `/* ... */` comments, write block comments with the
|
|
Packit |
8fb591 |
leading asterisk on each line. You may put the `/*` and `*/` on the same line as
|
|
Packit |
8fb591 |
comment text if you prefer.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
/*
|
|
Packit |
8fb591 |
* comment text
|
|
Packit |
8fb591 |
*/
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
### Functions
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Put the return type, function name, and the braces that surround the function's
|
|
Packit |
8fb591 |
code on separate lines, all starting in column 0.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
static int
|
|
Packit |
8fb591 |
foo(int arg)
|
|
Packit |
8fb591 |
{
|
|
Packit |
8fb591 |
...
|
|
Packit |
8fb591 |
}
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
When you need to put the function parameters on multiple lines, start new line
|
|
Packit |
8fb591 |
at column after the opening parenthesis from the initial line.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
static int
|
|
Packit |
8fb591 |
my_function(struct my_struct *p1, struct another_struct *p2,
|
|
Packit |
8fb591 |
int size)
|
|
Packit |
8fb591 |
{
|
|
Packit |
8fb591 |
...
|
|
Packit |
8fb591 |
}
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
In the absence of good reasons for another order, the following parameter order
|
|
Packit |
8fb591 |
is preferred. One notable exception is that data parameters and their
|
|
Packit |
8fb591 |
corresponding size parameters should be paired.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
1. The primary object being manipulated, if any (equivalent to the "this"
|
|
Packit |
8fb591 |
pointer in C++).
|
|
Packit |
8fb591 |
2. Input-only parameters.
|
|
Packit |
8fb591 |
3. Input/output parameters.
|
|
Packit |
8fb591 |
4. Output-only parameters.
|
|
Packit |
8fb591 |
5. Status parameter.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Functions that destroy an instance of a dynamically-allocated type should accept
|
|
Packit |
8fb591 |
and ignore a null pointer argument. Code that calls such a function (including
|
|
Packit |
8fb591 |
the C standard library function `free()`) should omit a null-pointer check. We
|
|
Packit |
8fb591 |
find that this usually makes code easier to read.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
#### Function Prototypes
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Put the return type and function name on the same line in a function prototype:
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
static const struct int foo(int arg);
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
### Statements
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
- Indent each level of code with 4 spaces.
|
|
Packit |
8fb591 |
- Put single space between `if`, `while`, `for`, etc. statements and the
|
|
Packit |
8fb591 |
expression that follow them. On the other hand, function calls has no space
|
|
Packit |
8fb591 |
between the function name and opening parenthesis.
|
|
Packit |
8fb591 |
- Opening code block brace is kept at the same line with the `if`, `while`,
|
|
Packit |
8fb591 |
`for` or `switch` statements.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
if (a) {
|
|
Packit |
8fb591 |
x = exp(a);
|
|
Packit |
8fb591 |
} else {
|
|
Packit |
8fb591 |
return 1;
|
|
Packit |
8fb591 |
}
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
- Start switch's cases at the same column as the switch.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
switch (conn->state) {
|
|
Packit |
8fb591 |
case 0:
|
|
Packit |
8fb591 |
return "data found";
|
|
Packit |
8fb591 |
case 1:
|
|
Packit |
8fb591 |
return "data not found";
|
|
Packit |
8fb591 |
default:
|
|
Packit |
8fb591 |
return "unknown error";
|
|
Packit |
8fb591 |
}
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
- Do not put gratuitous parentheses around the expression in a return statement,
|
|
Packit |
8fb591 |
that is, write `return 0;` and not `return(0);`
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
### Types
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Use typedefs sparingly. Code is clearer if the actual type is visible at the
|
|
Packit |
8fb591 |
point of declaration. Do not, in general, declare a typedef for a struct, union,
|
|
Packit |
8fb591 |
or enum. Do not declare a typedef for a pointer type, because this can be very
|
|
Packit |
8fb591 |
confusing to the reader.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Use the `int<N>_t` and `uint<N>_t` types from `<stdint.h>` for exact-width
|
|
Packit |
8fb591 |
integer types. Use the `PRId<N>`, `PRIu<N>`, and `PRIx<N>` macros from
|
|
Packit |
8fb591 |
`<inttypes.h>` for formatting them with `printf()` and related functions.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Pointer declarators bind to the variable name, not the type name. Write
|
|
Packit |
8fb591 |
`int *x`, not `int* x` and definitely not `int * x`.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
### Expresions
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Put one space on each side of infix binary and ternary operators:
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
* / % + - << >> < <= > >= == != & ^ | && || ?: = += -= *= /= %= &= ^= |= <<= >>=
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
Do not put any white space around postfix, prefix, or grouping operators with
|
|
Packit |
8fb591 |
one exception - `sizeof`, see the note below.
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
() [] -> . ! ~ ++ -- + - * &
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
The "sizeof" operator is unique among C operators in that it accepts two very
|
|
Packit |
8fb591 |
different kinds of operands: an expression or a type. In general, prefer to
|
|
Packit |
8fb591 |
specify an expression
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
int *x = calloc(1, sizeof *x);
|
|
Packit |
8fb591 |
```
|
|
Packit |
8fb591 |
When the operand of sizeof is an expression, there is no need to parenthesize
|
|
Packit |
8fb591 |
that operand, and please don't. There is an exception to this rule when you need
|
|
Packit |
8fb591 |
to work with partially compatible structures:
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
```c
|
|
Packit |
8fb591 |
struct a_s {
|
|
Packit |
8fb591 |
uint8_t type;
|
|
Packit |
8fb591 |
}
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
struct b_s {
|
|
Packit |
8fb591 |
uint8_t type;
|
|
Packit |
8fb591 |
char *str;
|
|
Packit |
8fb591 |
}
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
struct c_s {
|
|
Packit |
8fb591 |
uint8_t type;
|
|
Packit |
8fb591 |
uint8_t *u8;
|
|
Packit |
8fb591 |
}
|
|
Packit |
8fb591 |
...
|
|
Packit |
8fb591 |
struct a_s *a;
|
|
Packit |
8fb591 |
|
|
Packit |
8fb591 |
switch (type) {
|
|
Packit |
8fb591 |
case 1:
|
|
Packit |
8fb591 |
a = (struct a_s *)calloc(1, sizeof(struct b_s));
|
|
Packit |
8fb591 |
break;
|
|
Packit |
8fb591 |
case 2:
|
|
Packit |
8fb591 |
a = (struct a_s *)calloc(1, sizeof(struct c_s));
|
|
Packit |
8fb591 |
break;
|
|
Packit |
8fb591 |
...
|
|
Packit |
8fb591 |
```
|