1# Checklist for making changes to `libpldm` 2 3## Definitions 4 5- **Error condition**: An invalid state reached at runtime, caused either by 6 resource exhaustion, or incorrect use of the library's public APIs and data 7 types. 8 9- **Invariant**: A condition in the library's implementation that must never 10 evaluate false. 11 12- **Public API**: Any definitions and declarations under `include/libpldm`. 13 14## Elaborations 15 16- Resource exhaustion is always an error condition and never an invariant 17 violation. 18 19- An invariant violation is always a programming failure of the library's 20 implementation, and never the result of incorrect use of the library's public 21 APIs (see error condition). 22 23- Corrollaries of the above two points: 24 25 - Incorrect use of public API functions is always an error condition, and is 26 dealt with by returning an error code. 27 28 - Incorrect use of static functions in the library's implementation is an 29 invariant violation which may be established using `assert()`. 30 31- `assert()` is the recommended way to demonstrate invariants are upheld. 32 33## Adding a new API 34 35- [ ] My new public `struct` definitions are _not_ marked 36 `__attribute__((packed))` 37 38- [ ] If my work interacts with the PLDM wire format, then I have done so using 39 the `msgbuf` APIs found in `src/msgbuf.h` (and under `src/msgbuf/`) to 40 minimise concerns around spatial memory safety and endian-correctness. 41 42- [ ] All my error conditions are handled by returning an error code to the 43 caller. 44 45- [ ] All my invariants are tested using `assert()`. 46 47- [ ] I have not used `assert()` to evaluate any error conditions without also 48 handling the error condition by returning an error code the the caller. 49 50 - Release builds of the library are configured with `assert()` disabled 51 (`-Db_ndebug=if-release`, which provides `-DNDEBUG` in `CFLAGS`). 52 53- [ ] If I've implemented a new function, then it returns a negative `errno` 54 value on error and not a PLDM completion code. 55 56 - [ ] The specific error values my function returns and their meaning in the 57 context of the function call are listed in the API documentation. 58 59- [ ] If I've added support for a new PLDM message type, then I've implemented 60 both the encoder and decoder for that message. Note this applies for both 61 request _and_ response message types. 62 63- [ ] My new function symbols are marked with `LIBPLDM_ABI_TESTING` in the 64 implementation 65 66- [ ] I've implemented test cases with reasonable branch coverage of each new 67 function I've added 68 69- [ ] I've guarded the test cases of functions marked `LIBPLDM_ABI_TESTING` so 70 that they are not compiled when the corresponding function symbols aren't 71 visible 72 73- [ ] If I've added support for a new message type, then my commit message 74 specifies all of: 75 76 - [ ] The relevant DMTF specification by its DSP number and title 77 - [ ] The relevant version of the specification 78 - [ ] The section of the specification that defines the message type 79 80- [ ] If my work impacts the public API of the library, then I've added an entry 81 to `CHANGELOG.md` describing my work 82 83## Stabilising an existing API 84 85- [ ] The API of interest is currently marked `LIBPLDM_ABI_TESTING` 86 87- [ ] My commit message links to a publicly visible patch that makes use of the 88 API 89 90- [ ] My commit updates the annotation from `LIBPLDM_ABI_TESTING` to 91 `LIBPLDM_ABI_STABLE` only for the function symbols demonstrated by the 92 patch linked in the commit message. 93 94- [ ] I've removed guards from the function's tests so they are always compiled 95 96- [ ] If I've updated the ABI dump, then I've used the OpenBMC CI container to 97 do so. 98 99## Updating an ABI dump 100 101Each of the following must succeed: 102 103- [ ] Enter the OpenBMC CI Docker container 104 - Approximately: 105 `docker run --cap-add=sys_admin --rm=true --privileged=true -u $USER -w $(pwd) -v $(pwd):$(pwd) -e MAKEFLAGS= -it openbmc/ubuntu-unit-test:2024-W21-ce361f95ff4fa669` 106- [ ] `CC=gcc CXX=g++; [ $(uname -m) = 'x86_64' ] && meson setup -Dabi=deprecated,stable build` 107- [ ] `meson compile -C build` 108- [ ] `./scripts/abi-dump-formatter < build/src/current.dump > abi/x86_64/gcc.dump` 109 110## Removing an API 111 112- [ ] If the function is marked `LIBPLDM_ABI_TESTING`, then I have removed it 113 114- [ ] If the function is marked `LIBPLDM_ABI_STABLE`, then I have changed the 115 annotation to `LIBPLDM_ABI_DEPRECATED` and left it in-place. 116 117- [ ] If the function is marked `LIBPLDM_ABI_DEPRECATED`, then I have removed it 118 only after satisfying myself that each of the following is true: 119 120 - [ ] There are no known users of the function left in the community 121 - [ ] There has been at least one tagged release of `libpldm` subsequent to 122 the API being marked deprecated 123 124## Testing my changes 125 126Each of the following must succeed when executed in order. Note that to avoid 127[googletest bug #4232][googletest-issue-4232] you must avoid using GCC 12 128(shipped in Debian Bookworm). 129 130[googletest-issue-4232]: https://github.com/google/googletest/issues/4232 131 132- [ ] `meson setup -Dabi-compliance-check=disabled build` 133- [ ] `meson compile -C build && meson test -C build` 134 135- [ ] `meson configure --buildtype=release build` 136- [ ] `meson compile -C build && meson test -C build` 137 138- [ ] `meson configure --buildtype=debug build` 139- [ ] `meson configure -Dabi=deprecated,stable build` 140- [ ] `meson compile -C build && meson test -C build` 141