xref: /relibc/CONTRIBUTING.md (revision 74d0b24939d3cca85a155b5a6fcde8b7fc79090e)
192e1127bSTom Almeida# Contributing
292e1127bSTom Almeida
392e1127bSTom Almeida## Table of contents
492e1127bSTom Almeida1. [What to do](#what-to-do)
592e1127bSTom Almeida2. [Code style](#code-style)
692e1127bSTom Almeida3. [Sending merge requests](#sending-merge-requests)
792e1127bSTom Almeida4. [Writing tests](#writing-tests)
892e1127bSTom Almeida5. [Running tests](#running-tests)
992e1127bSTom Almeida
1092e1127bSTom AlmeidaMaintaining a libc is tough work, and we'd love some help!
1192e1127bSTom Almeida
1292e1127bSTom Almeida## What to do
1392e1127bSTom Almeida
1492e1127bSTom AlmeidaFor now, we are still trying to get full libc compatibility before we move on to
1592e1127bSTom Almeidaany optimisation.
1692e1127bSTom Almeida
1792e1127bSTom Almeida- We currently have a number of unimplemented functions. Search for
1892e1127bSTom Almeida    `unimplemented!()` and hop right in!
1992e1127bSTom Almeida- If you notice any missing functionality, feel free to add it in
2092e1127bSTom Almeida
2192e1127bSTom Almeida## Code style
2292e1127bSTom Almeida
2392e1127bSTom AlmeidaWe have a `rustfmt.toml` in the root directory of relibc. Please run `./fmt.sh`
2492e1127bSTom Almeidabefore sending in any merge requests as it will automatically format your code.
2592e1127bSTom Almeida
2692e1127bSTom AlmeidaWith regards to general style:
2792e1127bSTom Almeida
2892e1127bSTom Almeida### Where applicable, prefer using references to raw pointers
2992e1127bSTom Almeida
3092e1127bSTom AlmeidaThis is most obvious when looking at `stdio` functions. If raw pointers were
3192e1127bSTom Almeidaused instead of references, then the resulting code would be significantly
3292e1127bSTom Almeidauglier. Instead try to check for pointer being valid with `pointer::as_ref()`
3392e1127bSTom Almeidaand `pointer::as_mut()` and then immediately use those references instead.
3492e1127bSTom Almeida
3592e1127bSTom AlmeidaInternal functions should always take references.
3692e1127bSTom Almeida
3792e1127bSTom Almeida### Use the c types exposed in our platform module instead of Rust's inbuilt integer types
3892e1127bSTom Almeida
3992e1127bSTom AlmeidaThis is so we can guarantee that everything works across platforms. While it is
4092e1127bSTom Almeidagenerally accepted these days that an `int` has 32 bits (which matches against
4192e1127bSTom Almeidaan `i32`), some platforms have `int` as having 16 bits, and others have long as
4292e1127bSTom Almeidabeing 32 bits instead of 64. If you use the types in platform, then we can
4392e1127bSTom Almeidaguarantee that your code will "just work" should we port relibc to a different
4492e1127bSTom Almeidaarchitecture.
4592e1127bSTom Almeida
4692e1127bSTom Almeida### Use our internal functions
4792e1127bSTom Almeida
4892e1127bSTom AlmeidaIf you need to use a C string, don't reinvent the wheel. We have functions in
4992e1127bSTom Almeidathe platform module that convert C strings to Rust slices.
5092e1127bSTom Almeida
5192e1127bSTom AlmeidaWe also have structures that wrap files, wrap writable strings, and wrap various
5292e1127bSTom Almeidaother commonly used things that you should use instead of rolling your own.
5392e1127bSTom Almeida
5492e1127bSTom Almeida## Sending merge requests
5592e1127bSTom Almeida
5692e1127bSTom AlmeidaIf you have sent us a merge request, first of all, thanks for taking your time
5792e1127bSTom Almeidato help us!
5892e1127bSTom Almeida
5992e1127bSTom AlmeidaThe first thing to note is that we do most of our development on our
6092e1127bSTom Almeida[GitLab server](https://gitlab.redox-os.org/redox-os/relibc), and as such it is
6192e1127bSTom Almeidapossible that none of the maintainers will see your merge request if it is
6292e1127bSTom Almeidaopened on GitHub.
6392e1127bSTom Almeida
6492e1127bSTom AlmeidaIn your merge request, please put in the description:
6592e1127bSTom Almeida- What functions (if any) have been implemented or changed
6692e1127bSTom Almeida- The rationale behind your merge request (e.g. why you thought this change was
6792e1127bSTom Almeida    required. If you are just implementing some functions, you can ignore this)
6892e1127bSTom Almeida- Any issues that are related to the merge request
6992e1127bSTom Almeida
7092e1127bSTom AlmeidaWe have CI attached to our GitLab instance, so all merge requests are checked to
7192e1127bSTom Almeidamake sure that they are tested before they are merged. Please write tests for
7292e1127bSTom Almeidathe functions that you add/change and test locally on your own machine
7392e1127bSTom Almeida***before*** submitting a merge request.
7492e1127bSTom Almeida
7592e1127bSTom Almeida## Writing tests
7692e1127bSTom Almeida
7792e1127bSTom AlmeidaEvery function that gets written needs to have a test in C in order to make sure
7892e1127bSTom Almeidait works as intended. Here are a few guidelines for writing good tests.
7992e1127bSTom Almeida
8092e1127bSTom Almeida### Ensure that any literals you have are mapped to variables instead of being directly passed to a function.
8192e1127bSTom Almeida
8292e1127bSTom AlmeidaSometimes compilers take literals put into libc functions and run them
8392e1127bSTom Almeidainternally during compilation, which can cause some false positives.  All tests
8492e1127bSTom Almeidaare compiled with `-fno-builtin`, which theoretically solves this issue, but
8592e1127bSTom Almeidajust in case, it'd be a good idea to map inputs to variables.
8692e1127bSTom Almeida
8792e1127bSTom Almeida```c
8892e1127bSTom Almeida#include "string.h"
8992e1127bSTom Almeida#include "stdio.h"
9092e1127bSTom Almeida
91*ff874c87STibor Nagyint main(void) {
9292e1127bSTom Almeida    // Don't do this
9392e1127bSTom Almeida    printf("%d\n", strcspn("Hello", "Hi"));
9492e1127bSTom Almeida
9592e1127bSTom Almeida    // Do this
9692e1127bSTom Almeida    char *first = "Hello";
9792e1127bSTom Almeida    char *second = "Hi";
9892e1127bSTom Almeida    printf("%d\n", strcspn(first, second));
9992e1127bSTom Almeida}
10092e1127bSTom Almeida```
10192e1127bSTom Almeida
10292e1127bSTom Almeida### Ensure your tests cover every section of code.
10392e1127bSTom Almeida
10492e1127bSTom AlmeidaWhat happens if a string in `strcmp()` is shorter than the other string? What
10592e1127bSTom Almeidahappens if the first argument to `strcspn()` is longer than the second string?
10692e1127bSTom AlmeidaIn order to make sure that all functions work as expected, we ask that any tests
10792e1127bSTom Almeidacover as much of the code that you have written as possible.
10892e1127bSTom Almeida
10992e1127bSTom Almeida## Running tests
11092e1127bSTom Almeida
11192e1127bSTom AlmeidaRunning tests is an important part in trying to find bugs. Before opening a
11292e1127bSTom Almeidamerge request, we ask that you test on your own machine to make sure there are
11392e1127bSTom Almeidano regressions.
11492e1127bSTom Almeida
11592e1127bSTom AlmeidaYou can run tests with `make test` in the root directory of relibc to compile
11692e1127bSTom Almeidarelibc, compile the tests and run them. This *will* print a lot of output to
11792e1127bSTom Almeidastdout, so be warned!
11892e1127bSTom Almeida
11992e1127bSTom AlmeidaYou can test against verified correct output with `make verify` in the tests
12092e1127bSTom Almeidadirectory. You will need to manually create the correct output and put it in the
12192e1127bSTom Almeidatests/expected directory. Running any `make` commands in the tests directory
12292e1127bSTom Almeidawill ***not*** rebuild relibc, so you'll need to go back to the root directory
12392e1127bSTom Almeidaif you need to rebuild relibc.
124