|
Packit |
aea12f |
# GnuTLS -- Information about our contribution rules and coding style
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Anyone is welcome to contribute to GnuTLS. You can either take up
|
|
Packit |
aea12f |
tasks from our [planned list](https://gitlab.com/gnutls/gnutls/milestones),
|
|
Packit |
aea12f |
or suprise us with enhancement we didn't plan for. In all cases be prepared
|
|
Packit |
aea12f |
to defend and justify your enhancements, and get through few rounds
|
|
Packit |
aea12f |
of changes.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
We try to stick to the following rules, so when contributing please
|
|
Packit |
aea12f |
try to follow them too.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Git commits:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Note that when contributing code you will need to assert that the contribution is
|
|
Packit |
aea12f |
in accordance to the "Developer's Certificate of Origin" as found in the
|
|
Packit |
aea12f |
file [DCO.txt](doc/DCO.txt).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
To indicate that, make sure that your contributions (patches or merge requests),
|
|
Packit |
aea12f |
contain a "Signed-off-by" line, with your real name and e-mail address.
|
|
Packit |
aea12f |
To automate the process use "git am -s" to produce patches and/or set the
|
|
Packit |
aea12f |
a template to simplify this process, as follows.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
$ cp devel/git-template ~/.git-template
|
|
Packit |
aea12f |
[edit]
|
|
Packit |
aea12f |
$ git config commit.template ~/.git-template
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Test suite:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
New functionality should be accompanied by a test case which verifies
|
|
Packit |
aea12f |
the correctness of GnuTLS' operation on successful use of the new
|
|
Packit |
aea12f |
functionality, as well as on fail cases. The GnuTLS test suite is run on "make check"
|
|
Packit |
aea12f |
on every system GnuTLS is installed, except for the tests/suite part
|
|
Packit |
aea12f |
which is only run during development.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
For testing functionality of gnutls we use two test unit testing frameworks:
|
|
Packit |
aea12f |
1. The gnutls testing framework as in [utils.h](tests/utils.h), usually for high level
|
|
Packit |
aea12f |
tests such as testing a client against a server. See [set_x509_key_mem.c](tests/set_x509_key_mem.c).
|
|
Packit |
aea12f |
2. The cmocka unit testing framework, for unit testing of functions
|
|
Packit |
aea12f |
or interfaces. See [dtls-sliding-window.c](tests/dtls-sliding-window.c).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Certificates for testing purposes are available at [cert-common.h](tests/cert-common.h).
|
|
Packit |
aea12f |
Note that we do not regenerate test certificates when they expire, but
|
|
Packit |
aea12f |
we rather fix the test's time using datefudge or gnutls_global_set_time_function().
|
|
Packit |
aea12f |
For example, see [x509cert-tl.c](tests/x509cert-tl.c).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# File names:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Files are split to directories according to the subsystem
|
|
Packit |
aea12f |
they belong to. Examples are x509/, minitasn1/, openpgp/,
|
|
Packit |
aea12f |
opencdk/ etc. The files in the root directory related
|
|
Packit |
aea12f |
to the main TLS protocol implementation.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# C dialect:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
While parts of GnuTLS were written in older dialects, new code
|
|
Packit |
aea12f |
in GnuTLS are expected to conform to C99. Exceptions could be made
|
|
Packit |
aea12f |
for C99 features that are not supported in popular platforms on a
|
|
Packit |
aea12f |
case by case basis.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Indentation style:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
In general, use [the Linux kernel coding style](https://www.kernel.org/doc/html/latest/process/coding-style.html).
|
|
Packit |
aea12f |
You may indent the source using GNU indent, e.g. "indent -linux *.c".
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Commenting style
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
In general for documenting new code we prefer self-documented code to comments. That is:
|
|
Packit |
aea12f |
- Meaningful function and macro names
|
|
Packit |
aea12f |
- Short functions which do a single thing
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
That does not mean that no comments are allowed, but that when they are
|
|
Packit |
aea12f |
used, they are used to document something that is not obvious, or the protocol
|
|
Packit |
aea12f |
expectations. Though we haven't followed that rule strictly in the past, it
|
|
Packit |
aea12f |
should be followed on new code.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Function names:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
All the function names use underscore ```_```, to separate words,
|
|
Packit |
aea12f |
functions like ```gnutlsDoThat``` are not used. The exported function names
|
|
Packit |
aea12f |
usually start with the ```gnutls_``` prefix, and the words that follow
|
|
Packit |
aea12f |
specify the exact subsystem of gnutls that this function refers to.
|
|
Packit |
aea12f |
E.g. ```gnutls_x509_crt_get_dn```, refers to the X.509
|
|
Packit |
aea12f |
certificate parsing part of gnutls. Some of the used prefixes are the
|
|
Packit |
aea12f |
following.
|
|
Packit |
aea12f |
* ```gnutls_x509_crt_``` for the X.509 certificate part
|
|
Packit |
aea12f |
* ```gnutls_session_``` for the TLS session part (but this may be omited)
|
|
Packit |
aea12f |
* ```gnutls_handshake_``` for the TLS handshake part
|
|
Packit |
aea12f |
* ```gnutls_record_``` for the TLS record protocol part
|
|
Packit |
aea12f |
* ```gnutls_alert_``` for the TLS alert protocol part
|
|
Packit |
aea12f |
* ```gnutls_credentials_``` for the credentials structures
|
|
Packit |
aea12f |
* ```gnutls_global_``` for the global structures handling
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
All exported API functions must be listed in libgnutls.map
|
|
Packit |
aea12f |
in order to be exported.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Internal functions, i.e, functions that are not exported in the API but
|
|
Packit |
aea12f |
are used internally by multiple files, should be prefixed with an underscore.
|
|
Packit |
aea12f |
For example `_gnutls_handshake_begin()`.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Internal functions restricted to a file (static), or inline functions, should
|
|
Packit |
aea12f |
not use the `_gnutls` prefix for simplicity, e.g., `get_version()`.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Internal structures should not be exported. Especially pointers to
|
|
Packit |
aea12f |
internal data. Doing so harms future reorganization/rewrite of subsystems.
|
|
Packit |
aea12f |
They can however be used by unit tests in tests/ directory; in that
|
|
Packit |
aea12f |
case they should be part of the GNUTLS_PRIVATE_3_4 tag in libgnutls.map.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Header guards
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Each private C header file SHOULD have a header guard consisting of the
|
|
Packit |
aea12f |
project name and the file path relative to the project directory, all uppercase.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Example: `lib/srp.h` uses the header guard `GNUTLS_LIB_SRP_H`.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The header guard is used as first and last effective code in a header file,
|
|
Packit |
aea12f |
like e.g. in lib/srp.h:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
#ifndef GNUTLS_LIB_SRP_H
|
|
Packit |
aea12f |
#define GNUTLS_LIB_SRP_H
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
...
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
#endif /* GNUTLS_LIB_SRP_H */
|
|
Packit Service |
991b93 |
```
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The public header files follow a similar convention but use the relative
|
|
Packit |
aea12f |
install directory as template, e.g. `GNUTLS_GNUTLS_H` for `gnutls/gnutls.h`.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Introducing new functions / API
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Prior to introducing any new API consider all options to offer the same
|
|
Packit |
aea12f |
functionality without introducing a new function. The reason is that we want
|
|
Packit |
aea12f |
to avoid breaking the ABI, and thus we cannot typically remove any function
|
|
Packit |
aea12f |
that was added (though we have few exceptions). Since we cannot remove, it
|
|
Packit |
aea12f |
means that experimental APIs, or helper APIs that are not typically needed
|
|
Packit |
aea12f |
may become a burden to maintain in the future. That is, they may prevent
|
|
Packit |
aea12f |
a refactoring, or require to keep legacy code.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
As such, some questions to answer before adding a new API:
|
|
Packit |
aea12f |
* Is this API useful for a large class of applications, or is it limited
|
|
Packit |
aea12f |
to few?
|
|
Packit |
aea12f |
* If it is limited to few, can we work around that functionality without
|
|
Packit |
aea12f |
a new API?
|
|
Packit |
aea12f |
* Would that function be relevant in the future when a new protocol such TLS
|
|
Packit |
aea12f |
13.0 is made available? Would it harm the addition of a new protocol?
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The make rule 'abi-check' verifies that the ABI remained compatible since
|
|
Packit |
aea12f |
the last tagged release. It relies on the git tree and abi-compliance-checker.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The above do not apply to the C++ library; this library's ABI should not
|
|
Packit |
aea12f |
be considered stable.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Introducing new features / modifying behavior
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
When a new feature is introduced which may affect already deployed code,
|
|
Packit |
aea12f |
it must be disabled by default. For example a new TLS extension should be
|
|
Packit |
aea12f |
enabled when explicitly requested by the application. That can happen for
|
|
Packit |
aea12f |
example with a gnutls_init() flag.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The same should be followed when an existing function behavior is modified
|
|
Packit |
aea12f |
in a way that may break existing applications which use the API in a
|
|
Packit |
aea12f |
reasonable way. If the existing function allows flags, then a new flag
|
|
Packit |
aea12f |
should be introduced to enable the new behavior.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
When it is necessary, or desireable to enable the new features by default
|
|
Packit |
aea12f |
(e.g., TLS1.3 introduction), the "next" releases should be used (and
|
|
Packit |
aea12f |
introduced if necessary), to allow the modification to be tested for an
|
|
Packit Service |
991b93 |
extended amount of time (see the [Release policy](RELEASES.md)).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# API documentation
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
When introducing a new API, we provide the function documentation as
|
|
Packit |
aea12f |
inline comments, in a way that it can be used to generate a man-page
|
|
Packit |
aea12f |
and be included in our manual. For that we use gnome-style comments
|
|
Packit |
aea12f |
as in the example below. The detailed form is documented on `doc/scripts/gdoc`.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
/**
|
|
Packit |
aea12f |
* gnutls_init:
|
|
Packit |
aea12f |
* @session: is a pointer to a #gnutls_session_t type.
|
|
Packit |
aea12f |
* @flags: indicate if this session is to be used for server or client.
|
|
Packit |
aea12f |
*
|
|
Packit |
aea12f |
* This function initializes the provided session. Every
|
|
Packit |
aea12f |
* session must be initialized before use, and must be deinitialized
|
|
Packit |
aea12f |
* after used by calling gnutls_deinit().
|
|
Packit |
aea12f |
*
|
|
Packit |
aea12f |
* @flags can be any combination of flags from %gnutls_init_flags_t.
|
|
Packit |
aea12f |
*
|
|
Packit |
aea12f |
* Note that since version 3.1.2 this function enables some common
|
|
Packit |
aea12f |
* TLS extensions such as session tickets and OCSP certificate status
|
|
Packit |
aea12f |
* request in client side by default. To prevent that use the %GNUTLS_NO_EXTENSIONS
|
|
Packit |
aea12f |
* flag.
|
|
Packit |
aea12f |
*
|
|
Packit |
aea12f |
* Returns: %GNUTLS_E_SUCCESS on success, or a negative error code.
|
|
Packit |
aea12f |
**/
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Constructed types:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The constructed types in gnutls always have the ```gnutls_``` prefix.
|
|
Packit |
aea12f |
Definitions, value defaults and enumerated values should be in
|
|
Packit |
aea12f |
capitals. E.g. ```GNUTLS_CIPHER_3DES_CBC```.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Structures should have the ```_st``` suffix in their name even
|
|
Packit |
aea12f |
if they are a typedef. One can use the sizeof() on types with
|
|
Packit |
aea12f |
```_st``` as suffix to get the structure's size.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Other constructed types should have the ```_t``` suffix. A pointer
|
|
Packit |
aea12f |
to a structure also has the ```_t``` suffix.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Function parameters:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The gnutls functions accept parameters in the order:
|
|
Packit |
aea12f |
1. Input parameters
|
|
Packit |
aea12f |
2. Output parameters
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
When data and size are expected as input, a const gnutls_datum_t structure
|
|
Packit |
aea12f |
should be used (or more precisely a pointer to the structure).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
When data pointer and size are to be returned as output, a gnutls_datum_t
|
|
Packit |
aea12f |
structure should be used.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
When output is to be copied to caller an array of fixed data should be
|
|
Packit |
aea12f |
provided.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Callback function parameters:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Callback functions should be avoided, if this is possible.
|
|
Packit |
aea12f |
Callbacks that refer to a TLS session should include the
|
|
Packit |
aea12f |
current session as a parameter, in order for the called function to
|
|
Packit |
aea12f |
be able to retrieve the data associated with the session.
|
|
Packit |
aea12f |
This is not always done though -- see the push/pull callbacks.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Return values:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Functions in gnutls return an int type, when possible. In that
|
|
Packit |
aea12f |
case 0 should be returned in case of success, or maybe a positive
|
|
Packit |
aea12f |
value, if some other indication is needed.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
A negative value always indicates failure. All the available
|
|
Packit |
aea12f |
error codes are defined in gnutls.h and a description
|
|
Packit |
aea12f |
is available in gnutls_errors.c
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Functions which are intended to return a boolean value should return
|
|
Packit |
aea12f |
a type of bool, and it is recommended to contain the string '_is_'
|
|
Packit |
aea12f |
on its function name; e.g.,
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
bool _gnutls_is_not_prehashed();
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
That allows the distinguishing functions that return negative errors
|
|
Packit |
aea12f |
from boolean functions to both the developer and the compiler. Note
|
|
Packit |
aea12f |
that in the past the 'unsigned' type was used to distinguish boolean functions
|
|
Packit |
aea12f |
and several of these still exist.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
## Selecting the right return value
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
When selecting the return value for a TLS protocol parsing function
|
|
Packit |
aea12f |
a suggested approach is to check which alert fits best on that error
|
|
Packit |
aea12f |
(see `alert.c`), and then select from the error codes which are mapped
|
|
Packit |
aea12f |
to that alert (see `gnutls_error_to_alert()`). For more generic parsing
|
|
Packit |
aea12f |
errors consider using the `GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER`.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Usage of assert()
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The assert() macro --not to be confused with gnutls_assert()-- is used
|
|
Packit |
aea12f |
exceptionally on impossible situations to assist static analysis tools.
|
|
Packit |
aea12f |
That is, it should be used when the static analyzer used in CI (currently
|
|
Packit |
aea12f |
clang analyzer), detects an error which is on an impossible situation.
|
|
Packit |
aea12f |
In these cases assert() is used to rule out that case.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
For example in the situation where a pointer is known to be non-null,
|
|
Packit |
aea12f |
but the static analyzer cannot rule it out, we use code like the following:
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
assert(ptr != NULL);
|
|
Packit |
aea12f |
ptr->deref = 3;
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Since GnuTLS is a library no other uses of assert() macro are acceptable.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The NDEBUG macro is not used in GnuTLS compilation, so the assert() macros
|
|
Packit |
aea12f |
are always active.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Gnulib
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The directories `gl/`, `src/gl/` and `lib/unistring` contain gnulib files
|
|
Packit |
aea12f |
copied/created by `./bootstrap`. Gnulib is a portability source code library
|
|
Packit |
aea12f |
to handle API or behavior incompatibilities between target systems.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
To take advantage of the latest gnulib files, we have to update the
|
|
Packit |
aea12f |
`gnulib/` submodule from time to time:
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
$ make glimport
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Note that the gnulib library in `gl/` is used by the GnuTLS library
|
|
Packit |
aea12f |
and is kept separate from the gnulib used by the GnuTLS tools because
|
|
Packit |
aea12f |
of license issues, and also to prevent any gnulib networking modules
|
|
Packit |
aea12f |
from entering the library (gnulib networking re-implements the windows
|
|
Packit |
aea12f |
network stack and causes issues to gnutls applications running on windows).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Compiler warnings
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The compiler prints warnings for several reasons; these warnings are
|
|
Packit |
aea12f |
also not constant in time, different versions of the same compiler may
|
|
Packit |
aea12f |
warn about different issues.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
In GnuTLS we enable as many as possible warnings available in the compiler
|
|
Packit |
aea12f |
via configure.ac. On certain cases however we silence or disable warnings
|
|
Packit |
aea12f |
and the following subsections go case by case.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
## Switch unintended fall-through warnings
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
These we silence by using the macro FALLTHROUGH under a switch
|
|
Packit |
aea12f |
statement which intentionally falls through. Example:
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
switch (session->internals.recv_state) {
|
|
Packit |
aea12f |
case RECV_STATE_DTLS_RETRANSMIT:
|
|
Packit |
aea12f |
ret = _dtls_retransmit(session);
|
|
Packit |
aea12f |
if (ret < 0)
|
|
Packit |
aea12f |
return gnutls_assert_val(ret);
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
session->internals.recv_state = RECV_STATE_0;
|
|
Packit |
aea12f |
FALLTHROUGH;
|
|
Packit |
aea12f |
case RECV_STATE_0:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
_dtls_async_timer_check(session);
|
|
Packit |
aea12f |
return 1;
|
|
Packit |
aea12f |
}
|
|
Packit |
aea12f |
```
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Symbol and library versioning
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The library uses the libtool versioning system, which in turn
|
|
Packit |
aea12f |
results to a soname bump on incompatible changes. That is described
|
|
Packit |
aea12f |
in [hooks.m4](m4/hooks.m4). Despite its complexity that system is
|
|
Packit |
aea12f |
only sufficient to distinguish between versions of the library that
|
|
Packit |
aea12f |
have broke ABI (i.e., soname bump occurred).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Today however, soname versioning isn't sufficient. Symbol versioning
|
|
Packit |
aea12f |
as provided by [libgnutls.map](lib/libgnutls.map) have several
|
|
Packit |
aea12f |
advantages.
|
|
Packit |
aea12f |
* they allow for symbol clashing between different gnutls library
|
|
Packit |
aea12f |
versions being in the same address space.
|
|
Packit |
aea12f |
* they allow installers to detect the library version used for
|
|
Packit |
aea12f |
an application utilizing a specific symbol
|
|
Packit |
aea12f |
* the allow introducing multiple versions of a symbol a la libc,
|
|
Packit |
aea12f |
keeping the semantics of old functions while introducing new.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
As such for every symbol introduced on a particular version, we
|
|
Packit |
aea12f |
create an entry in libgnutls.map based on the version and containing
|
|
Packit |
aea12f |
the new symbols. For example, if in version 3.6.2 we introduce symbol
|
|
Packit |
aea12f |
```gnutls_xyz```, the entry would be:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
GNUTLS_3_6_2 {
|
|
Packit |
aea12f |
global:
|
|
Packit |
aea12f |
gnutls_xyz;
|
|
Packit |
aea12f |
} GNUTLS_3_6_1;
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
where ```GNUTLS_3_6_1``` is the last version that symbols were introduced,
|
|
Packit |
aea12f |
and indicates a dependency of 3.6.2 to symbols of 3.6.1.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Note that when the soname version is bumped, i.e., the ABI is broken
|
|
Packit |
aea12f |
all the previous symbol versions should be combined into a single. For
|
|
Packit |
aea12f |
example on the 3.4.0 soname bump, all symbols were put under the
|
|
Packit |
aea12f |
GNUTLS_3_4 version.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Backporting new symbols to an old version which is soname compatible
|
|
Packit |
aea12f |
is not allowed (can cause quite some problems). Backporting symbols
|
|
Packit |
aea12f |
to an incompatible soname version is allowed, but must ensure that
|
|
Packit |
aea12f |
the symbol version used for the backported symbol version is distinct from
|
|
Packit |
aea12f |
the original library symbol version. E.g., if symbol ```gnutls_xyz```
|
|
Packit |
aea12f |
with version GNUTLS_3_6_3 is backported on gnutls 3.3.15, it should
|
|
Packit |
aea12f |
use version GNUTLS_3_3_15.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Auto-generated files:
|
|
Packit |
aea12f |
Several parts of the documentation and the command line tools parameters
|
|
Packit |
aea12f |
files (.def) are auto-generated. Normally when introducing new functions,
|
|
Packit |
aea12f |
or adding new command line options to tools you need to run 'make
|
|
Packit |
aea12f |
files-update', review the output (when feasible) and commit it separately,
|
|
Packit |
aea12f |
e.g., with a message: "auto-generated files update".
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Guile bindings:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Parts of the Guile bindings, such as types (aka. "SMOBs"), enum values,
|
|
Packit |
aea12f |
constants, are automatically generated. This is handled by the modules
|
|
Packit |
aea12f |
under `guile/modules/gnutls/build/'; these modules are only used at
|
|
Packit |
aea12f |
build-time and are not installed.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The Scheme variables they generate (e.g., constants, type predicates,
|
|
Packit |
aea12f |
etc.) are exported to user programs through `gnutls.scm' and
|
|
Packit |
aea12f |
`gnutls/extra.scm', both of which are installed.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
For instance, when adding/removing/renaming enumerates or constants,
|
|
Packit |
aea12f |
two things must be done:
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
1. Update the enum list in `build/enums.scm' (currently dependencies
|
|
Packit |
aea12f |
are not tracked, so you have to run "make clean all" in `guile/'
|
|
Packit |
aea12f |
after).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
2. Update the export list of `gnutls.scm' (or `extra.scm').
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
Note that, for constants and enums, "schemefied" names are used, as
|
|
Packit |
aea12f |
noted under the "Guile API Conventions" node of the manual.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Automated testing
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
GnuTLS primarily relies on gitlab-ci which is configured in .gitlab-ci.yml
|
|
Packit |
aea12f |
file in the repository. The goal is to have a test suite which runs for
|
|
Packit |
aea12f |
every new merge request prior to merging. There are no particular rules for
|
|
Packit |
aea12f |
the test targets, except for them being reliable and running in a reasonable
|
|
Packit |
aea12f |
timeframe (~1 hour).
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
# Reviewing code
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
A review as part of the gitlab merge requests, is a way to prevent errors due to
|
|
Packit |
aea12f |
these guidelines not being followed, e.g., verify there is a reasonable test suite,
|
|
Packit |
aea12f |
and whether it covers reasonably the new code, that the function naming is
|
|
Packit |
aea12f |
consistent with these guidelines, as well as check for obvious mistakes in the new
|
|
Packit |
aea12f |
code.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
The intention is to keep reviews lightweight, and rely on CI for tasks such
|
|
Packit |
aea12f |
as compiling and testing code and features.
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
A proposed checklist to assist such reviews follows.
|
|
Packit |
aea12f |
* [ ] Any issues marked for closing are addressed
|
|
Packit |
aea12f |
* [ ] There is a test suite reasonably covering new functionality or modifications
|
|
Packit |
aea12f |
* [ ] Function naming, parameters, return values, types, etc., are consistent and according to `CONTRIBUTION.md`
|
|
Packit |
aea12f |
* [ ] This feature/change has adequate documentation added
|
|
Packit |
aea12f |
* [ ] No obvious mistakes in the code
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
|
|
Packit |
aea12f |
[Guidelines to consider when reviewing.](https://github.com/thoughtbot/guides/tree/master/code-review)
|