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/
65 - [To be a better programmer, write little proofs in your head - Matthew
66   Prast][matthew-prast-little-proofs]
68 [matthew-prast-little-proofs]:
69   https://the-nerve-blog.ghost.io/to-be-a-better-programmer-write-little-proofs-in-your-head/
73 - [The C programming language - C Standards Committee][c-language]
75 [c-language]: https://www.c-language.org/
77 - [SEI CERT C Coding Standard][sei-cert-c-coding-standard]
79 [sei-cert-c-coding-standard]:
82 - [Common Weakness Enumeration (CWE) - Software
83   Development][common-weakness-enumeration-sw]
85 [common-weakness-enumeration-sw]:
90 - **Error condition**: An invalid state reached at runtime, caused either by
94 - **Invariant**: A condition in the library's implementation that must never
97 - **Public API**: Any definitions and declarations under `include/libpldm`.
99 - **Wire format**: Any message structure defined in the DMTF PLDM protocol
104 - Resource exhaustion is always an error condition and never an invariant
107 - An invariant violation is always a programming failure of the library's
111 - Corollaries of the above two points:
112   - Incorrect use of public API functions is always an error condition, and is
115   - Incorrect use of static functions in the library's implementation is an
118 - `assert()` is the recommended way to demonstrate invariants are upheld.
125 ---
127 ---
128 stateDiagram-v2
130     [*] --> Testing: Add
131     Testing --> Testing: Change
132     Testing --> [*]: Remove
133     Testing --> Stable: Stabilise
134     Stable --> Deprecated: Deprecate
135     Deprecated --> [*]: Remove
138 The ABI of the library produced by the build is controlled using the `abi` meson
143 | ----------- | --------------------------------- |
144 | Production  | `-Dabi=deprecated,stable`         |
145 | Maintenance | `-Dabi=stable`                    |
146 | Development | `-Dabi=deprecated,stable,testing` |
151 off of deprecated APIs by constraining the library ABI to the stable category.
152 This will force the compiler identify any call-sites that try to link against
153 deprecated symbols.
171 ### Marking functions as testing, stable or deprecated
174 translation units) to mark functions as testing, stable or deprecated:
231 meson setup ... -Dlibpldm:abi=deprecated,stable,testing ...
236 - [ ] My commit message subject is prefixed with the name of the impacted
239 - [ ] My commit message describes testing practices only if the discussion is
241   - The description must contain enough information for someone else to
244   - A section on testing should not be added if there was no specific testing
251 - [ ] All publicly exposed macros, types and functions relating to the PLDM
254   - The only (temporary) exception are the `encode_*()` and `decode_*()`
257 - [ ] All publicly exposed macros, types and functions relating to the library
260 - [ ] All `pldm_`-prefixed symbols must also name the related specification. For
264 - [ ] All enum members must be prefixed with the type name
268 - [ ] If I've added support for a new PLDM message type, then I've defined both
270   - This applies for both request _and_ response message types.
272 - [ ] I've designed my APIs so their implementation does not require heap
274   - Prefer [defining iterators][libpldm-iterator] over the message buffer to
275     extract sub-structures from variable-length messages. Iterators avoid both
278     provided with an on-stack struct containing the extracted sub-structure.
280 [libpldm-iterator]:
283 - [ ] My `encode_*()` APIs exchange with the caller the size of the destination
284       buffer and the length of the encoded data using an in-out buffer length
287 - [ ] My new public message codec functions take a `struct` representing the
289   - Function prototypes must _not_ decompose the message to individual
291     type-safe. This is especially true for message decoding functions which must
292     use pointers for out-parameters, where it has often become ambiguous whether
295 - [ ] Each new `struct` I've defined is used in at least one new function I've
298 - [ ] My new public `struct` definitions are _not_ marked
301 - [ ] My new public `struct` definitions do _not_ define a flexible array
303   - [ ] It's contained in an `#ifndef __cplusplus` macro guard, as flexible
306   - [ ] I've implemented an accessor function so the array base pointer can be
309   - [ ] It is defined as per the C17 specification by omitting the length[^1]
310     - Note: Any array defined with length 1 is _not_ a flexible array, and any
314   - [ ] I've annotated the flexible array member with `LIBPLDM_CC_COUNTED_BY()`
317     [C17 draft specification][c17-draft-standard], 6.7.2.1 Structure and union
320 [c17-draft-standard]:
321 …https://web.archive.org/web/20181230041359/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_upda…
325 - [ ] My new function symbols are marked with `LIBPLDM_ABI_TESTING` in the
328 - [ ] I've guarded the test cases of functions marked `LIBPLDM_ABI_TESTING` so
334 - [ ] All my error conditions are handled by returning an error code to the
337 - [ ] All my invariants are tested using `assert()`.
339 - [ ] I have not used `assert()` to evaluate any error conditions without also
341   - Release builds of the library are configured with `assert()` disabled
342     (`-Db_ndebug=if-release`, which provides `-DNDEBUG` in `CFLAGS`).
344 - [ ] My new APIs return negative `errno` values on error and not PLDM
346   - [ ] The specific error values my function returns and their meaning in the
351 - [ ] If my work interacts with the PLDM wire format, then I have done so using
353       minimise concerns around spatial memory safety and endian-correctness.
355 - [ ] I've used `goto` to clean up resources acquired prior to encountering an
357   - Replication of resource cleanup across multiple error paths is error-prone,
360 - [ ] I've released acquired resources in stack-order
361   - This should be the case regardless of whether we're in the happy path at the
364 - [ ] I've declared variables in [reverse-christmas-tree (inverted pyramid)
365       order][hisham-make-pyramids] in any block scopes I've added or changed.
367 [hisham-make-pyramids]:
368 …hive.org/web/20220404224603/https://hisham.hm/2018/06/16/when-listing-repeated-things-make-pyramid…
372 - [ ] I've implemented test cases with reasonable branch coverage of each new
377 - [ ] If I've added support for a new message type, then my commit message
379   - [ ] The relevant DMTF specification by its DSP number and title
380   - [ ] The relevant version of the specification
381   - [ ] The section of the specification that defines the message type
383 - [ ] If my work impacts the public API of the library, then I've added an entry
386 ### OEM/vendor-specific APIs
388 - [ ] I've documented the wire format for all OEM messages under
391 - [ ] I've added public OEM API declarations and definitions under
395 - [ ] I've implemented the public OEM APIs under `src/oem/${OEM_NAME}/`
397 - [ ] I've implemented the OEM API tests under `tests/oem/${OEM_NAME}/`
403 option, and the `meson.build` files updated throughout the tree to guard
408 - [ ] The API of interest is currently marked `LIBPLDM_ABI_TESTING`
410 - [ ] My commit message links to a publicly visible patch that makes use of the
413 - [ ] My commit updates the annotation from `LIBPLDM_ABI_TESTING` to
417 - [ ] I've removed guards from the function's tests so they are always compiled
419 - [ ] If I've updated the ABI dump, then I've used the OpenBMC CI container to
424 To update the ABI dump you'll need to build an appropriate OpenBMC CI container
426 openbmc/docs repository][openbmc-docs-local-ci]. You can list your locally built
429 [openbmc-docs-local-ci]:
430   https://github.com/openbmc/docs/blob/master/testing/local-ci-build.md
435 export OPENBMC_CI_IMAGE=openbmc/ubuntu-unit-test:2024-W21-ce361f95ff4fa669
442   --cap-add=sys_admin \
443   --rm=true \
444   --privileged=true \
445   -u $USER \
446   -w $(pwd) \
447   -v $(pwd):$(pwd) \
448   -e MAKEFLAGS= \
449   -t $OPENBMC_CI_IMAGE \
450   ./scripts/abi-dump-updater
455 - [ ] If the function is marked `LIBPLDM_ABI_TESTING`, then I have removed it
457 - [ ] If the function is marked `LIBPLDM_ABI_STABLE`, then I have changed the
458       annotation to `LIBPLDM_ABI_DEPRECATED` and left it in-place.
459   - [ ] I have updated the ABI dump, or will mark the change as WIP until it has
462 - [ ] If the function is marked `LIBPLDM_ABI_DEPRECATED`, then I have removed it
464   - [ ] There are no known users of the function left in the community
465   - [ ] There has been at least one tagged release of `libpldm` subsequent to
466         the API being marked deprecated
477 - [ ] Only the name of the function has changed. None of its behaviour has
480 - [ ] Both the new and the old functions are declared in the public headers
482 - [ ] I've aliased the old function name to the new function name via the
483       `libpldm_deprecated_aliases` list in `meson.build`
485 - [ ] I've added a [semantic patch][coccinelle] to migrate users from the old
490 - [ ] I've updated the ABI dump to capture the rename, or will mark the change
495 - [ ] My change fixing the bug includes a [Fixes tag][linux-kernel-fixes-tag]
498 [linux-kernel-fixes-tag]:
499   https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
501 - [ ] My change fixing the bug includes test cases demonstrating that the bug is