openbmc/libpldm/CONTRIBUTING.md

5 - [Good Practices in Library Design, Implementation, and Maintenance - Ulrich
10 - [How Do I Make This Hard to Misuse? - Rusty Russell][rusty-api-scale-good]
12 [rusty-api-scale-good]: https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html
14 - [What If I Don't Actually Like My Users? - Rusty Russell][rusty-api-scale-bad]
16 [rusty-api-scale-bad]: https://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html
18 - [Red flags that indicate questionable quality - Lennart
19   Poettering][poettering-library-red-flags]
21 [poettering-library-red-flags]:
24 - [Not sure if this is a gcc bug or some weird corner of UB or what... - Andrew
25   Zonenberg][azonenberg-packed-struct]
27 [azonenberg-packed-struct]: https://ioc.exchange/@azonenberg/112535511250395148
29 - [The Good, the Bad, and the Weird - Trail of Bits
30   Blog][trail-of-bits-weird-machines]
32 [trail-of-bits-weird-machines]:
33   https://blog.trailofbits.com/2018/10/26/the-good-the-bad-and-the-weird/
35 - [Logic for Programmers - Hillel Wayne][logic-for-programmers]
37 [logic-for-programmers]: https://leanpub.com/logic
39 - [Parse, don’t validate - Alexis King][alexis-king-parse-dont-validate]
41 [alexis-king-parse-dont-validate]:
42   https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/
44 - [The byte order fallacy - command center][command-center-byte-order-fallacy]
46 [command-center-byte-order-fallacy]:
47   https://commandcenter.blogspot.com/2012/04/byte-order-fallacy.html
49 - [what’s the most annoying thing about writing C - Jessica
50   Paquette][barrelshifter-annoying-c]
52 [barrelshifter-annoying-c]:
55 - [The Power of Ten: Rules for Developing Safety Critical Code - NASA/JPL
56   Laboratory for Reliable Software][nasa-reliable-code]
58 [nasa-reliable-code]:
59   https://www.cs.otago.ac.nz/cosc345/resources/nasa-10-rules.pdf
61 - [C Isn't A Programming Language Anymore - Aria Desires][aria-c-is-a-protocol]
63 [aria-c-is-a-protocol]: https://faultlore.com/blah/c-isnt-a-language/
67 - [The C programming language - C Standards Committee][c-language]
69 [c-language]: https://www.c-language.org/
71 - [SEI CERT C Coding Standard][sei-cert-c-coding-standard]
73 [sei-cert-c-coding-standard]:
76 - [Common Weakness Enumeration (CWE) - Software
77   Development][common-weakness-enumeration-sw]
79 [common-weakness-enumeration-sw]:
84 - **Error condition**: An invalid state reached at runtime, caused either by
88 - **Invariant**: A condition in the library's implementation that must never
91 - **Public API**: Any definitions and declarations under `include/libpldm`.
93 - **Wire format**: Any message structure defined in the DMTF PLDM protocol
98 - Resource exhaustion is always an error condition and never an invariant
101 - An invariant violation is always a programming failure of the library's
102   implementation, and never the result of incorrect use of the library's public
105 - Corollaries of the above two points:
106   - Incorrect use of public API functions is always an error condition, and is
109   - Incorrect use of static functions in the library's implementation is an
112 - `assert()` is the recommended way to demonstrate invariants are upheld.
119 ---
121 ---
122 stateDiagram-v2
124     [*] --> Testing: Add
125     Testing --> Testing: Change
126     Testing --> [*]: Remove
127     Testing --> Stable: Stabilise
128     Stable --> Deprecated: Deprecate
129     Deprecated --> [*]: Remove
137 | ----------- | --------------------------------- |
138 | Production  | `-Dabi=deprecated,stable`         |
139 | Maintenance | `-Dabi=stable`                    |
140 | Development | `-Dabi=deprecated,stable,testing` |
146 This will force the compiler identify any call-sites that try to link against
198 implementation bugs. It is just a promise that the prototype won't change
225 meson setup ... -Dlibpldm:abi=deprecated,stable,testing ...
232 - [ ] All publicly exposed macros, types and functions relating to the PLDM
235   - The only (temporary) exception are the `encode_*()` and `decode_*()`
238 - [ ] All publicly exposed macros, types and functions relating to the library
239       implementation must be prefixed with `libpldm_` or `LIBPLDM_`
241 - [ ] All `pldm_`-prefixed symbols must also name the related specification. For
245 - [ ] All enum members must be prefixed with the type name
249 - [ ] If I've added support for a new PLDM message type, then I've defined both
251   - This applies for both request _and_ response message types.
253 - [ ] I've designed my APIs so their implementation does not require heap
255   - Prefer [defining iterators][libpldm-iterator] over the message buffer to
256     extract sub-structures from variable-length messages. Iterators avoid both
257     requiring heap allocation in the implementation or splitting the API to
259     provided with an on-stack struct containing the extracted sub-structure.
261 [libpldm-iterator]:
264 - [ ] My new public message codec functions take a `struct` representing the
266   - Function prototypes must _not_ decompose the message to individual
268     type-safe. This is especially true for message decoding functions which must
269     use pointers for out-parameters, where it has often become ambiguous whether
272 - [ ] Each new `struct` I've defined is used in at least one new function I've
275 - [ ] My new public `struct` definitions are _not_ marked
278 - [ ] My new public `struct` definitions do _not_ define a flexible array
280   - [ ] It's contained in an `#ifndef __cplusplus` macro guard, as flexible
283   - [ ] I've implemented an accessor function so the array base pointer can be
286   - [ ] It is defined as per the C17 specification by omitting the length[^1]
287     - Note: Any array defined with length 1 is _not_ a flexible array, and any
291   - [ ] I've annotated the flexible array member with `LIBPLDM_CC_COUNTED_BY()`
294     [C17 draft specification][c17-draft-standard], 6.7.2.1 Structure and union
297 [c17-draft-standard]:
298   https://web.archive.org/web/20181230041359/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf
302 - [ ] My new function symbols are marked with `LIBPLDM_ABI_TESTING` in the
303       implementation
305 - [ ] I've guarded the test cases of functions marked `LIBPLDM_ABI_TESTING` so
311 - [ ] All my error conditions are handled by returning an error code to the
314 - [ ] All my invariants are tested using `assert()`.
316 - [ ] I have not used `assert()` to evaluate any error conditions without also
318   - Release builds of the library are configured with `assert()` disabled
319     (`-Db_ndebug=if-release`, which provides `-DNDEBUG` in `CFLAGS`).
321 - [ ] My new APIs return negative `errno` values on error and not PLDM
323   - [ ] The specific error values my function returns and their meaning in the
326 ### Implementation
328 - [ ] If my work interacts with the PLDM wire format, then I have done so using
330       minimise concerns around spatial memory safety and endian-correctness.
332 - [ ] I've used `goto` to clean up resources acquired prior to encountering an
334   - Replication of resource cleanup across multiple error paths is error-prone,
337 - [ ] I've released acquired resources in stack-order
338   - This should be the case regardless of whether we're in the happy path at the
341 - [ ] I've declared variables in [reverse-christmas-tree (inverted pyramid)
342       order][hisham-make-pyramids] in any block scopes I've added or changed.
344 [hisham-make-pyramids]:
345   https://web.archive.org/web/20220404224603/https://hisham.hm/2018/06/16/when-listing-repeated-things-make-pyramids/
349 - [ ] I've implemented test cases with reasonable branch coverage of each new
354 - [ ] If I've added support for a new message type, then my commit message
356   - [ ] The relevant DMTF specification by its DSP number and title
357   - [ ] The relevant version of the specification
358   - [ ] The section of the specification that defines the message type
360 - [ ] If my work impacts the public API of the library, then I've added an entry
363 ### OEM/vendor-specific APIs
365 - [ ] I've documented the wire format for all OEM messages under
368 - [ ] I've added public OEM API declarations and definitions under
372 - [ ] I've implemented the public OEM APIs under `src/oem/${OEM_NAME}/`
374 - [ ] I've implemented the OEM API tests under `tests/oem/${OEM_NAME}/`
385 - [ ] The API of interest is currently marked `LIBPLDM_ABI_TESTING`
387 - [ ] My commit message links to a publicly visible patch that makes use of the
390 - [ ] My commit updates the annotation from `LIBPLDM_ABI_TESTING` to
394 - [ ] I've removed guards from the function's tests so they are always compiled
396 - [ ] If I've updated the ABI dump, then I've used the OpenBMC CI container to
403 openbmc/docs repository][openbmc-docs-local-ci]. You can list your locally built
406 [openbmc-docs-local-ci]:
407   https://github.com/openbmc/docs/blob/master/testing/local-ci-build.md
412 export OPENBMC_CI_IMAGE=openbmc/ubuntu-unit-test:2024-W21-ce361f95ff4fa669
419   --cap-add=sys_admin \
420   --rm=true \
421   --privileged=true \
422   -u $USER \
423   -w $(pwd) \
424   -v $(pwd):$(pwd) \
425   -e MAKEFLAGS= \
426   -t $OPENBMC_CI_IMAGE \
427   ./scripts/abi-dump-updater
432 - [ ] If the function is marked `LIBPLDM_ABI_TESTING`, then I have removed it
434 - [ ] If the function is marked `LIBPLDM_ABI_STABLE`, then I have changed the
435       annotation to `LIBPLDM_ABI_DEPRECATED` and left it in-place.
436   - [ ] I have updated the ABI dump, or will mark the change as WIP until it has
439 - [ ] If the function is marked `LIBPLDM_ABI_DEPRECATED`, then I have removed it
441   - [ ] There are no known users of the function left in the community
442   - [ ] There has been at least one tagged release of `libpldm` subsequent to
454 - [ ] Only the name of the function has changed. None of its behaviour has
457 - [ ] Both the new and the old functions are declared in the public headers
459 - [ ] I've aliased the old function name to the new function name via the
462 - [ ] I've added a [semantic patch][coccinelle] to migrate users from the old
467 - [ ] I've updated the ABI dump to capture the rename, or will mark the change
470 ## Fixing Implementation Defects
472 - [ ] My change fixing the bug includes a [Fixes tag][linux-kernel-fixes-tag]
475 [linux-kernel-fixes-tag]:
476   https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
478 - [ ] My change fixing the bug includes test cases demonstrating that the bug is