Revision tags: v0.11.0, v0.10.0 |
|
#
8f3b13c4 |
| 16-Oct-2024 |
Matt Johnston <matt@codeconstruct.com.au> |
msgbuf: pldm_msgbuf_peek_remaining and pldm_msgbuf_skip
These can be used to populate a msgbuf by passing a buffer/length to an external function that doesn't use msgbuf. pldm_msgbuf_peek_remaining(
msgbuf: pldm_msgbuf_peek_remaining and pldm_msgbuf_skip
These can be used to populate a msgbuf by passing a buffer/length to an external function that doesn't use msgbuf. pldm_msgbuf_peek_remaining() provides a buffer and length to pass, and pldm_msgbuf_skip() is called after the data has been written into that buffer, to update the used length.
Change-Id: I55012e2e724842665f5317e252c9e3ae81803936 Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
show more ...
|
#
5d4f7b57 |
| 12-Dec-2024 |
Matt Johnston <matt@codeconstruct.com.au> |
msgbuf: Add pldm_msgbuf_destroy_used()
Used when inserting int a msgbuf, returns the total length that has been used.
Change-Id: I981c6b8a1fd194387fdf8a6a3299c3811d33c622 Signed-off-by: Matt Johnst
msgbuf: Add pldm_msgbuf_destroy_used()
Used when inserting int a msgbuf, returns the total length that has been used.
Change-Id: I981c6b8a1fd194387fdf8a6a3299c3811d33c622 Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
show more ...
|
#
e8d8d338 |
| 28-Oct-2024 |
Matt Johnston <matt@codeconstruct.com.au> |
msgbuf: Add pldm_msgbuf_insert_uint64
Required for encoding firmware update non_functioning_component_bitmap
Change-Id: I3360b09150021bc30423bdc8de4936b6fa31092d Signed-off-by: Matt Johnston <matt@
msgbuf: Add pldm_msgbuf_insert_uint64
Required for encoding firmware update non_functioning_component_bitmap
Change-Id: I3360b09150021bc30423bdc8de4936b6fa31092d Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
show more ...
|
#
4f60fb77 |
| 22-Sep-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Bounds checks that satisfy GCC's analyzer
The intent is that there is no change in behavior, but that the code patterns better match the analyzer's expectations.
Change-Id: I58544aaf6b15209
msgbuf: Bounds checks that satisfy GCC's analyzer
The intent is that there is no change in behavior, but that the code patterns better match the analyzer's expectations.
Change-Id: I58544aaf6b15209e754059bf72a55dc9d63c9d61 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
830c1eb4 |
| 03-Oct-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Externalise error value conversion
We need to simplify the code to satisfy clang's analyzer, which seems to struggle with assumptions if the code exceeds some unknown complexity limit.
Spec
msgbuf: Externalise error value conversion
We need to simplify the code to satisfy clang's analyzer, which seems to struggle with assumptions if the code exceeds some unknown complexity limit.
Specifically, this does away with pldm_msgbuf_init_cc() and all the associated pldm_msgbuf_status() error translation machinery. All the call-sites are fixed up, with some additional safety checks put in place along the way.
I believe this change is viable because unless we're converting legacy API implementations to use msgbuf there's no additional trickery, and if we're converting existing implementations then care is required regardless. The change of approach has no impact on implementation of new APIs with msgbuf, as the current philosophy is that they should return negative errnos anyway.
As seems to be the case with this kind of work, the parameter register allocation seems to have been affected for a number of library APIs. These are listed in the changelog, and the ABI dump has been updated.
Finally, for msgbuf use in the test cases, all instances have been converted to use errnos in place of PLDM completion codes in the expectations. Hopefully there's no more malarky with PLDM completion code misuse in the future.
Change-Id: Id4a7366ee9f60fb991dfe84aa0bb5aadc9855fcc Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
e5f12538 |
| 30-Sep-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Improve type-specific ergonomics to match generic macros
Don't require that a pointer be passed. Rather, take the pointer inside the type-safe macro definition, and perform the void cast to
msgbuf: Improve type-specific ergonomics to match generic macros
Don't require that a pointer be passed. Rather, take the pointer inside the type-safe macro definition, and perform the void cast to avoid the alignment warning.
Change-Id: I5fbfc4a95591d2640595107e6f5fcae44a95950f Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
Revision tags: v0.9.1, v0.9.0 |
|
#
7939382f |
| 07-Aug-2024 |
Varsha Kaverappa <vkaverap@in.ibm.com> |
msgbuf: Allow pldm_msgbuf_span_required to accept NULL
Allow pldm_msgbuf_span_required to accept NULL as an argument so we can use this API to skip past data in the msg buffer which is not required
msgbuf: Allow pldm_msgbuf_span_required to accept NULL
Allow pldm_msgbuf_span_required to accept NULL as an argument so we can use this API to skip past data in the msg buffer which is not required and extract only the relevant data.
Change-Id: I08d233b8efe415732fb7c01c00a9925f04666fe2 Signed-off-by: Varsha Kaverappa <vkaverap@in.ibm.com>
show more ...
|
#
90bbe6c0 |
| 01-Sep-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
compiler: Provide LIBPLDM_CC_NONNULL{,_ARGS()}
This allows us to elide checks where they're not necessary, and warn people at compile-time when they're doing things they shouldn't.
Note that this c
compiler: Provide LIBPLDM_CC_NONNULL{,_ARGS()}
This allows us to elide checks where they're not necessary, and warn people at compile-time when they're doing things they shouldn't.
Note that this comes with an apparent ABI break. abi-compliance-checker reports:
``` platform.h, libpldm.so.0.8.0 [−] decode_sensor_op_data ( uint8_t const* sensor_data, size_t sensor_data_length, uint8_t* present_op_state, uint8_t* previous_op_state ) Change: The parameter previous_op_state became passed in r8 register instead of rcx. Effect Applications will read the wrong memory block instead of the parameter value. ```
It's unclear to me why. The signature hasn't changed, but how the implementation tests the parameter values has.
Change-Id: Ie8d8bc1641280522532d9b4764bf07c64b1921c8 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
0a1be3cb |
| 11-Aug-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Harden pldm_msgbuf_{insert,extract}_array()
Review of some proposed APIs suggested that correct use of the pldm_msgbuf_{insert,extract}_array() helpers was more difficult that it should be.
msgbuf: Harden pldm_msgbuf_{insert,extract}_array()
Review of some proposed APIs suggested that correct use of the pldm_msgbuf_{insert,extract}_array() helpers was more difficult that it should be. In the three-parameter form, it was too tempting to provide the length to extract as parsed out of a PLDM message. The intended use was that the length parameter represented the length of the user-provided data buffer.
Instead, move to a four-parameter form, provide reasonable documentation for how these APIs should be used, fix all the call-sites, and deprecate some existing unsafe APIs.
Change-Id: If58e5574600e80b354f383554283c4eda5d7234c Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
cb569bc5 |
| 01-Sep-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
compiler: Provide LIBPLDM_CC_ALWAYS_INLINE
Using it through the code-base reduces some of the source noise, and gives us a way to control the definition going forward.
Change-Id: I27e76cbae5c45f0ef
compiler: Provide LIBPLDM_CC_ALWAYS_INLINE
Using it through the code-base reduces some of the source noise, and gives us a way to control the definition going forward.
Change-Id: I27e76cbae5c45f0efd64b01fb9a8b243a6c8e65d Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
860a43d9 |
| 22-Aug-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
compiler: Provide LIBPLDM_CC_UNUSED
Ensure that we have __has_attribute available, and then further ensure that the unused attribute is provided. Once satisfied, define LIBPDLM_CC_UNUSED and replace
compiler: Provide LIBPLDM_CC_UNUSED
Ensure that we have __has_attribute available, and then further ensure that the unused attribute is provided. Once satisfied, define LIBPDLM_CC_UNUSED and replace raw use of __attribute__((unused)).
Change-Id: I2433039297d5fdedb8b8d99b30e73e4542d9069f Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
2b440d4c |
| 24-Jul-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Ensure memmem() is correctly typed and visible where required
To ensure memmem() is visible _GNU_SOURCE needs to be defined early, at least before any system headers are included. Define it
msgbuf: Ensure memmem() is correctly typed and visible where required
To ensure memmem() is visible _GNU_SOURCE needs to be defined early, at least before any system headers are included. Define it in the build flags as clang-tidy will re-order includes based on vibes rather than dependencies. Finally, clean up the remaining compiler warnings by dropping the unnecessary casts.
Note that _GNU_SOURCE implies _DEFAULT_SOURCE, so we drop the latter:
> Since glibc 2.19, defining _GNU_SOURCE also has the effect of > implicitly defining _DEFAULT_SOURCE. Before glibc 2.20, defining > _GNU_SOURCE also had the effect of implicitly defining _BSD_SOURCE > and _SVID_SOURCE.
https://www.man7.org/linux/man-pages/man7/feature_test_macros.7.html
Fixes: #12 Fixes: 1523778d2739 ("msgbuf: Add pldm_msgbuf_span_string_utf16()") Change-Id: I9206f7616740790a89366762cce11d3045471b97 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
56f73f95 |
| 07-Jul-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Add pldm_msgbuf_copy_string_utf16()
Safely copy a NUL-terminated UTF16-{BE,LE} string between msgbuf instances.
Change-Id: If96df9598f17ac771d75f0831be270c5e0139578 Signed-off-by: Andrew Je
msgbuf: Add pldm_msgbuf_copy_string_utf16()
Safely copy a NUL-terminated UTF16-{BE,LE} string between msgbuf instances.
Change-Id: If96df9598f17ac771d75f0831be270c5e0139578 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
1523778d |
| 02-Jul-2024 |
Thu Nguyen <thu@os.amperecomputing.com> |
msgbuf: Add pldm_msgbuf_span_string_utf16()
Add pldm_msgbuf_span_string_utf16 API to return the start pointer of the utf16 string in message buffer. The API also returns the UTF16 string length in t
msgbuf: Add pldm_msgbuf_span_string_utf16()
Add pldm_msgbuf_span_string_utf16 API to return the start pointer of the utf16 string in message buffer. The API also returns the UTF16 string length in terms of bytes, including the NUL terminator.
``` __attribute__((always_inline)) static inline int pldm_msgbuf_span_string_utf16(struct pldm_msgbuf *ctx, void **cursor, size_t *length) ```
The `cursor` and `length` are optional. Input NULL to `cursor` and `length` will cause the message buffer cursor points to remaining data. The caller can ignore `length` option by input NULL if they don't care about the size of utf16 string.
Change-Id: I1fc2865a21d9925e49416531b85212b3b07dc37a Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com> Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
8b879600 |
| 07-Jul-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Add pldm_msgbuf_copy_string_ascii()
Safely copy a NUL-terminated string between msgbuf instances.
Change-Id: I224dc3f5bbd55fd9d4727ab0de065d5253ee0bea Signed-off-by: Andrew Jeffery <andrew@
msgbuf: Add pldm_msgbuf_copy_string_ascii()
Safely copy a NUL-terminated string between msgbuf instances.
Change-Id: I224dc3f5bbd55fd9d4727ab0de065d5253ee0bea Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
9c83d681 |
| 02-Jul-2024 |
Thu Nguyen <thu@os.amperecomputing.com> |
msgbuf: Add pldm_msgbuf_span_string_ascii()
Add pldm_msgbuf_span_string_ascii() API to find the start of the ascii string in the message buffer.
``` pldm_msgbuf_span_string_ascii(struct pldm_msgbuf
msgbuf: Add pldm_msgbuf_span_string_ascii()
Add pldm_msgbuf_span_string_ascii() API to find the start of the ascii string in the message buffer.
``` pldm_msgbuf_span_string_ascii(struct pldm_msgbuf *ctx, void **cursor, size_t *length) ```
The API returns the start pointer of ascii string in the message buffer and length of that ascii string includes Terminator. The `cursor` and `length` are optional. Input NULL to `cursor` and `length` will cause the message buffer cursor points to remaining data. The caller can ignore `length` option by input NULL if they don't care about the size of ascii string.
Change-Id: I4a73b7425ee1e4e5621eb16de6e16189efdf202b Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com> Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
1c57144d |
| 07-Jul-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Generalize array extraction and insertion
Build the type-safe and generic behavior on top of memcpy() via a "private" helper that takes a void pointer.
Change-Id: Iedb8e9237c780735d4cac41fe
msgbuf: Generalize array extraction and insertion
Build the type-safe and generic behavior on top of memcpy() via a "private" helper that takes a void pointer.
Change-Id: Iedb8e9237c780735d4cac41fe0a723c3751c64ce Signed-off-by: Chris Wang <chris.wang.wiwynn@gmail.com> Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
9e3a5d45 |
| 17-Jun-2024 |
Manojkiran Eda <manojkiran.eda@gmail.com> |
Fix spelling mistakes using codespell
This commit corrects various spelling mistakes throughout the repository. The corrections were made automatically using `codespell`[1] tool.
[1]: https://githu
Fix spelling mistakes using codespell
This commit corrects various spelling mistakes throughout the repository. The corrections were made automatically using `codespell`[1] tool.
[1]: https://github.com/codespell-project/codespell
Change-Id: I25415165df192cfc3bd1405aca81bfa5bf2f7a63 Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
show more ...
|
#
d861a681 |
| 03-Jun-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
dsp: base: Rework {un,}pack_pldm_header() error handling
The current preference is that library APIs return negative errno values to signal implementation errors. That doesn't jive with existing sta
dsp: base: Rework {un,}pack_pldm_header() error handling
The current preference is that library APIs return negative errno values to signal implementation errors. That doesn't jive with existing stable APIs returning PLDM completion codes, so provide a means to translate between the two.
The first users are the {un,}pack_pldm_header() functions.
Change-Id: I7b7cb97a1d8b96ec0fec1c0a5fbd8503da834d86 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
Revision tags: v0.8.0 |
|
#
c8df31c1 |
| 21-May-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Add error code personalities
libpldm is in a bit of a transitional period with respect to returned error codes. A historical choice was to return PLDM completion codes from the library API t
msgbuf: Add error code personalities
libpldm is in a bit of a transitional period with respect to returned error codes. A historical choice was to return PLDM completion codes from the library API to indicate errors. This is unfortunate because we're now constrained to errors that are specified by the PLDM protocol, which is much less expressive than the set of errors that might be produced by a run-time environment for the library.
The choice going forward is to return C's errno codes. However at this point we step on another rake in the libpldm design, which is that some internal data structures are very much the wire format of corresponding PLDM messages (such as the PDR repository implementation). Working with wire-format buffers is most safely done via the msgbuf APIs, however we then hit the conflict of different error code styles in various parts of the API surface.
Do a bit of surgery to provide different error code personalities for msgbuf, such that the caller can pick the style of error code they need it to return to maintain consistency.
Note that like the previous patch marking all msgbuf APIs as __attribute__((always_inline)), the rework here makes another small impact on the argument register allocation of several stable APIs. The ABI dump is updated accordingly.
Change-Id: Id59c39c5c822f514f546dab88575317071a97c96 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
76712f69 |
| 22-May-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Always inline all functions
The intent of the msgbuf APIs is that they are glorified wrappers around singular load/store sequences. Given this, mark all of them as `__attribute__((always_inl
msgbuf: Always inline all functions
The intent of the msgbuf APIs is that they are glorified wrappers around singular load/store sequences. Given this, mark all of them as `__attribute__((always_inline))`.
Note that this apparently impacts the library ABI of the following functions:
- `decode_get_state_sensor_readings_resp` - `decode_platform_event_message_req` - `decode_pldm_pdr_repository_change_record_data` - `decode_pldm_pdr_repository_chg_event_data` - `decode_sensor_op_data` - `decode_set_event_receiver_resp` - `decode_state_sensor_data`
On x86_64 the argument register selection is rearranged, for example:
```diff 'Reg' => { - '2' => 'rbx' + '0' => 'rdi', + '1' => 'rsi', + '2' => 'rdx' }, 'Return' => '100', 'ShortName' => 'decode_set_event_receiver_resp' ```
I'm yet to understand how we ended up with argument 2 being passed in `rbx`, as the psABI[1][2] v1.0 does not define `rbx` as an argument-passing register (see Figure 3.4 on page 26). Possibly because it's callee-saved it can be exploited this way. The previously recorded registers of other functions appear to have the same concern. By contrast, `rdi`, `rsi` and `rdx` are defined as the 1st, 2nd and 3rd argument-passing registers respectively, which with respect to the function at hand seems reasonable.
[1]: https://gitlab.com/x86-psABIs/x86-64-ABI/-/jobs/artifacts/master/raw/x86-64-ABI/abi.pdf?job=build [2]: https://gitlab.com/x86-psABIs/x86-64-ABI
Change-Id: I38c2d36d254bbd54012848b2b5c5ab52c0f04003 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
909bf7c2 |
| 03-May-2024 |
Varsha Kaverappa <vkaverap@in.ibm.com> |
msgbuf: Add copy API
pldm_msgbuf_copy API allows copy of data from one msg buffer to another. This was done earlier with a pldm_msgbuf_extract() followed by pldm_msgbuf_insert().
Change-Id: I159792
msgbuf: Add copy API
pldm_msgbuf_copy API allows copy of data from one msg buffer to another. This was done earlier with a pldm_msgbuf_extract() followed by pldm_msgbuf_insert().
Change-Id: I159792f726916761894aefb0a8795f1f0dc84114 Signed-off-by: Varsha Kaverappa <vkaverap@in.ibm.com>
show more ...
|
#
2ff8cf89 |
| 17-May-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Remove use of ssize_t for overflow tracking
There are a few concerns with the use of ssize_t in this context:
1. It's defined by POSIX and not C, and I'd prefer we not require POSIX conc
msgbuf: Remove use of ssize_t for overflow tracking
There are a few concerns with the use of ssize_t in this context:
1. It's defined by POSIX and not C, and I'd prefer we not require POSIX concepts where we can avoid it 2. ssize_t is defined over [-1, SSIZE_MAX] - it is not defined to have the range of a regular signed type.
The source of both these statements is The Open Group Base Specifications Issue 7, 2018 edition. IEEE Std 1003.1-2017 (Revision of IEEE Std 1003.1-2008)
The second point directly contradicts how I was trying to use ssize_t in the msgbuf implementation. As a result, switch the type of `remaining` to intmax_t. Usually intmax_t is a problem child, but it's not used in any public API, and it has the semantics I wanted by contrast to the definition of ssize_t.
Note that we add assert() calls where we know the value of remaining must be negative. Without the addition of the `assert()` calls in the underflow checks, clang-analyzer gets tripped up by not being able to prove `INTMAX_MIN + (intmax_t)sizeof(uint16_t) < 0`:
``` ../src/platform.c:17:18: error: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors] 17 | if (ctx->length + sizeof(*ctx) < lower) { | ^ ../src/platform.c:2445:6: note: 'rc' is 0 2445 | if (rc) { | ^~ ../src/platform.c:2445:2: note: Taking false branch 2445 | if (rc) { | ^ ../src/platform.c:2449:7: note: Calling 'pldm_msgbuf_extract_value_pdr_hdr' 2449 | rc = pldm_msgbuf_extract_value_pdr_hdr(buf, &hdr); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/msgbuf/platform.h:17:2: note: Calling 'pldm__msgbuf_extract_uint16' 17 | pldm_msgbuf_extract(ctx, hdr->length); | ^ ../src/msgbuf/../msgbuf.h:517:2: note: expanded from macro 'pldm_msgbuf_extract' 517 | _Generic((dst), \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 518 | uint8_t: pldm__msgbuf_extract_uint8, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 519 | int8_t: pldm__msgbuf_extract_int8, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 520 | uint16_t: pldm__msgbuf_extract_uint16, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 521 | int16_t: pldm__msgbuf_extract_int16, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 522 | uint32_t: pldm__msgbuf_extract_uint32, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 523 | int32_t: pldm__msgbuf_extract_int32, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 524 | real32_t: pldm__msgbuf_extract_real32)(ctx, (void *)&(dst)) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/msgbuf/../msgbuf.h:341:7: note: 'ctx' is non-null 341 | if (!ctx || !ctx->cursor || !dst) { | ^~~ ../src/msgbuf/../msgbuf.h:341:6: note: Left side of '||' is false 341 | if (!ctx || !ctx->cursor || !dst) { | ^ ../src/msgbuf/../msgbuf.h:341:20: note: Field 'cursor' is non-null 341 | if (!ctx || !ctx->cursor || !dst) { | ^ ../src/msgbuf/../msgbuf.h:341:6: note: Left side of '||' is false 341 | if (!ctx || !ctx->cursor || !dst) { | ^ ../src/msgbuf/../msgbuf.h:341:31: note: 'dst' is non-null 341 | if (!ctx || !ctx->cursor || !dst) { | ^~~ ../src/msgbuf/../msgbuf.h:341:2: note: Taking false branch 341 | if (!ctx || !ctx->cursor || !dst) { | ^ ../src/msgbuf/../msgbuf.h:347:2: note: Taking true branch 347 | if (ctx->remaining < INTMAX_MIN + (intmax_t)sizeof(ldst)) { | ^ ../src/msgbuf/../msgbuf.h:348:3: note: Returning without writing to '*dst' 348 | return PLDM_ERROR_INVALID_LENGTH; | ^ ../src/msgbuf/platform.h:17:2: note: Returning from 'pldm__msgbuf_extract_uint16' 17 | pldm_msgbuf_extract(ctx, hdr->length); | ^ ../src/msgbuf/../msgbuf.h:517:2: note: expanded from macro 'pldm_msgbuf_extract' 517 | _Generic((dst), \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 518 | uint8_t: pldm__msgbuf_extract_uint8, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 519 | int8_t: pldm__msgbuf_extract_int8, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 520 | uint16_t: pldm__msgbuf_extract_uint16, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 521 | int16_t: pldm__msgbuf_extract_int16, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 522 | uint32_t: pldm__msgbuf_extract_uint32, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 523 | int32_t: pldm__msgbuf_extract_int32, \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 524 | real32_t: pldm__msgbuf_extract_real32)(ctx, (void *)&(dst)) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/msgbuf/platform.h:19:2: note: Returning without writing to 'hdr->length' 19 | return pldm_msgbuf_validate(ctx); | ^ ../src/platform.c:2449:7: note: Returning from 'pldm_msgbuf_extract_value_pdr_hdr' 2449 | rc = pldm_msgbuf_extract_value_pdr_hdr(buf, &hdr); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../src/platform.c:2450:6: note: 'rc' is 0 2450 | if (rc) { | ^~ ../src/platform.c:2450:2: note: Taking false branch 2450 | if (rc) { | ^ ../src/platform.c:2454:7: note: Calling 'pldm_platform_pdr_hdr_validate' 2454 | rc = pldm_platform_pdr_hdr_validate( | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2455 | &hdr, PLDM_PDR_NUMERIC_EFFECTER_PDR_MIN_LENGTH, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2456 | pdr_data_length); | ~~~~~~~~~~~~~~~~ ../src/platform.c:17:18: note: The left operand of '+' is a garbage value 17 | if (ctx->length + sizeof(*ctx) < lower) { | ~~~~~~~~~~~ ^ ```
Change-Id: Idbe5a14455ad677a39c8f535eddd9c2ce471c783 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
07febdbb |
| 16-May-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Rework detection of invalid memory regions
From Annex J.2 of N2176 (C17 draft specification):
> Addition or subtraction of a pointer into, or just beyond, an array > object and an integer t
msgbuf: Rework detection of invalid memory regions
From Annex J.2 of N2176 (C17 draft specification):
> Addition or subtraction of a pointer into, or just beyond, an array > object and an integer type produces a result that does not point into, > or just beyond, the same array object (6.5.6).
Instead we can lean on uintptr_t from 7.20.1.4, and from there the defined behavior of unsigned overflow.
Change-Id: Ia1b47b87efeb9c96057d294a3e38e90bfdba5386 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
#
66c7723a |
| 23-Apr-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Enable pldm_msgbuf_extract() into packed members
`pldm_msgbuf_extract()` should work correctly regardless of whether the `dst` argument is a member of a packed or padded struct.
To get ther
msgbuf: Enable pldm_msgbuf_extract() into packed members
`pldm_msgbuf_extract()` should work correctly regardless of whether the `dst` argument is a member of a packed or padded struct.
To get there while still achieving type safety we have to jump through some hoops. Commentary in the patch hopefully captures many of them, but a side-effect of the hoop-jumping is a couple of changes to ergonomics of the msgbuf API:
1. `pldm_msgbuf_extract()` no-longer requires that the `dst` argument be a pointer. Instead, it must be an lvalue, removing all the `&<lvalue>` noise from the call-sites.
2. However, unfortunately the generic extraction macro has been split in two. We now have:
2.1 `pldm_msgbuf_extract()`, and 2.2 `pldm_msgbuf_extract_p()`, for when the reference we already have for the `dst` object is a pointer and not an lvalue.
The split was necessary because I couldn't get GCC and Clang to play nice with differences required in the assignment expression for lvalue and pointer type-names in the one macro. Whilst it causes a bunch of churn it isn't a great concern as the APIs are purely internal to the library implementation.
Change-Id: Ifc5440a5b838a48bb84c881ec334d9e145365edb Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au> Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>
show more ...
|