| commit | 953bc8c1a4e9f21981897d06b71159e403dfff1b | [log] [tgz] |
|---|---|---|
| author | Andrew Jeffery <andrew@aj.id.au> | Mon May 22 15:24:17 2023 +0930 |
| committer | Andrew Jeffery <andrew@aj.id.au> | Tue May 23 05:19:42 2023 +0000 |
| tree | 7ebe82765aaec62f2bc2294fa6bb08742c39651a | |
| parent | 7c25034015406b5859572ce7a550ad7a33c0ff6b [diff] |
libpldm: Enable API/ABI compliance checks
This catches breaks to the API and ABI of the library at compile time,
providing feedback to developers that they should probably not be doing
whatever it is they have done.
abi-compliance-checker is only enabled if tests are enabled and the
necessary external programs can be found. The check can be forcefully
disabled with `-Dabi-compliance-check=disabled`.
A further constraint is that compliance is only checked with GCC. There
are some minor issues with abi-compliance-checker if the new dump is
generated with a different compiler (e.g. clang) to the reference dump.
The GCC limitation will remain until I can find a comfortable way
forward.
Below is an example of a detected violation where an existing API is
changed:
```
$ git diff
diff --git a/include/libpldm/platform.h b/include/libpldm/platform.h
index ead3421af4f9..44654bc6f0c3 100644
--- a/include/libpldm/platform.h
+++ b/include/libpldm/platform.h
@@ -1385,7 +1385,7 @@ int encode_get_pdr_repository_info_resp(
const uint8_t *update_time, const uint8_t *oem_update_time,
uint32_t record_count, uint32_t repository_size,
uint32_t largest_record_size, uint8_t data_transfer_handle_timeout,
- struct pldm_msg *msg);
+ struct pldm_msg *msg, int test);
/** @brief Decode GetPDRRepositoryInfo response data
*
diff --git a/src/platform.c b/src/platform.c
index 5da538cfa36e..69e5f87b0e41 100644
--- a/src/platform.c
+++ b/src/platform.c
@@ -366,7 +366,7 @@ int encode_get_pdr_repository_info_resp(
const uint8_t *update_time, const uint8_t *oem_update_time,
uint32_t record_count, uint32_t repository_size,
uint32_t largest_record_size, uint8_t data_transfer_handle_timeout,
- struct pldm_msg *msg)
+ struct pldm_msg *msg, int test __attribute__((unused)))
{
if (msg == NULL) {
return PLDM_ERROR_INVALID_DATA;
diff --git a/tests/libpldm_platform_test.cpp b/tests/libpldm_platform_test.cpp
index 0b3370dec60e..6fc1dd005535 100644
--- a/tests/libpldm_platform_test.cpp
+++ b/tests/libpldm_platform_test.cpp
@@ -405,7 +405,7 @@ TEST(GetPDRRepositoryInfo, testGoodEncodeResponse)
auto rc = encode_get_pdr_repository_info_resp(
0, PLDM_SUCCESS, repositoryState, updateTime, oemUpdateTime,
recordCount, repositorySize, largestRecordSize,
- dataTransferHandleTimeout, response);
+ dataTransferHandleTimeout, response, 1);
EXPECT_EQ(rc, PLDM_SUCCESS);
struct pldm_pdr_repository_info_resp* resp =
@@ -436,7 +436,7 @@ TEST(GetPDRRepositoryInfo, testBadEncodeResponse)
auto rc = encode_get_pdr_repository_info_resp(
0, PLDM_SUCCESS, repositoryState, updateTime, oemUpdateTime,
recordCount, repositorySize, largestRecordSize,
- dataTransferHandleTimeout, nullptr);
+ dataTransferHandleTimeout, nullptr, 1);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
}
$ meson compile -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /usr/bin/ninja -C /mnt/host/andrew/home/andrew/src/openbmc/libpldm/build
ninja: Entering directory `/mnt/host/andrew/home/andrew/src/openbmc/libpldm/build'
[9/10] Generating abi-dump with a custom command
Detect public symbols
Reading debug-info
WARNING: a "Struct" type with no attributes detected in the DWARF dump (192)
ERROR: invalid debug_loc section of object, please fix your elf utils
ERROR: missed type id 13275
ERROR: missed type id 13632
ERROR: missed type id 11631
ERROR: missed type id 13540
ERROR: missed type id 702
ERROR: missed type id 60867
ERROR: missed type id 60369
ERROR: missed type id 30341
ERROR: missed type id 3570
ERROR: missed type id 3881
ERROR: missed type id 65373
ERROR: missed type id 66112
ERROR: missed type id 66916
Creating ABI dump
The object ABI has been dumped to:
current.dump
[10/10] Generating abi-compliance with a custom command
FAILED: abi-compliance
/usr/bin/abi-compliance-checker -l libpldm -old /mnt/host/andrew/home/andrew/src/openbmc/libpldm/abi/baseline.dump -new current.dump
Preparing, please wait ...
Comparing ABIs ...
Comparing APIs ...
Creating compatibility report ...
Binary compatibility: 99.8%
Source compatibility: 99.8%
Total binary compatibility problems: 1, warnings: 0
Total source compatibility problems: 1, warnings: 0
Report: compat_reports/libpldm/0.2.0_to_0.2.0/compat_report.html
ninja: build stopped: subcommand failed.
```
Change-Id: I72981ce48420eee970c3917059fbcbd350b00da9
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
This is a library which deals with the encoding and decoding of PLDM messages. It should be possible to use this library by projects other than OpenBMC, and hence certain constraints apply to it:
Source files are named according to the PLDM Type, for eg base.[h/c], fru.[h/c], etc.
Given a PLDM command "foo", the library will provide the following API: For the Requester function:
encode_foo_req() - encode a foo request decode_foo_resp() - decode a response to foo
For the Responder function:
decode_foo_req() - decode a foo request encode_foo_resp() - encode a response to foo
The library also provides API to pack and unpack PLDM headers.
Need meson and ninja. Alternatively, source an OpenBMC ARM/x86 SDK.
meson setup builddir && ninja -C builddir
The simplest way of running the tests is as described by the meson man page:
meson setup builddir && meson test -C builddir
This will support OEM or vendor-specific functions and semantic information. Following directory structure has to be used:
libpldm |---- include/libpldm | |---- oem/<oem_name>/libpldm | |----<oem based .h files> |---- src | |---- oem/<oem_name> | |----<oem based .c files> |---- tests | |---- oem/<oem_name> | |----<oem based test files>
<oem_name> - This folder must be created with the name of the OEM/vendor in lower case.
Header files & source files having the oem functionality for the libpldm library should be placed under the respective folder hierarchy as mentioned in the above figure. They must be adhering to the rules mentioned under the libpldm section above.
Once the above is done a meson option has to be created in libpldm/meson_options.txt with its mapped compiler flag to enable conditional compilation.
For consistency would recommend using "oem-<oem_name>".
The libpldm/meson.build and the corresponding source file(s) will need to incorporate the logic of adding its mapped compiler flag to allow conditional compilation of the code.
The pldm requester API's are present in src/requester folder and they are intended to provide API's to interact with the desired underlying transport layer to send/receive pldm messages.
NOTE : In the current state, the requester API's in the repository only works with specific transport mechanism & these are going to change in future & probably aren't appropriate to be writing code against.