pal: platform abstraction layer and strerror_r(3) wrapper #8390

altimeter-130ft · 2024-01-18T15:26:36Z

This PR is for the following changes:

Introduce the pal module, the platform abstraction layer.
Add flb_strerror_r(), the wrapper of strerror_r(3).

Fixed Issue

Background

The strerror_r(3) implementations of glibc and POSIX.1-2001 have the different signatures and usages, which causes a build error and/or an unexpected behaviour.

Please refer to Issue #8379 for the detailed analysis.

Changes

This fix implements flb_strerror_r(), the POSIX.1-2001-compliant wrapper of strerror_r(3) into pal, the new core module dedicated to encapsulate the platform dependency found on the common external features. The goal of pal is similar to the AC_LIBOBJ() macro of GNU Autoconf, except that pal holds all wrapper functions in a single source.

The wrapper functions in pal are intended for the Fluent Bit core and plugins only. The third-party libraries under lib must not call them, in order to keep their independency.

`pal` Source Files

src/flb_pal.c
include/fluent-bit/flb_pal.h

`pal` Internal Test Files

tests/internal/pal.c

New `pal` Function

int flb_strerror_r(int errnum, char *buf, size_t buflen);
The wrapper of strerror_r(3) with the signature and semantics of POSIX.1-2001. Available if strerror_r(3) exists in the platform, regardless from its implementation.

`flb_strerror_r()` Callers

The strerror_r(3) calls in the following files have been converted to flb_strerror_r():

src/flb_io.c
src/flb_log.c
src/flb_network.c

Tested Environments

Debian x86_64 12.2 (strerror_r(3) standard: glibc)
FreeBSD/amd64 14.0-RELEASE (strerror_r(3) standard: POSIX.1-2001)
- cmake: add LLVM coverage support #8376 included for the coverage report.

Testing

N/A Example configuration file for the change
The issue does not depend on a specific configuration.
Debug log output from testing the change
Attached Valgrind output that shows no leaks or memory corruption was found

Test Results

Debian x86_64 12.2: logs-fluent-bit-topic-pal-strerror_r-wrapper-github-pr-debian_x86_64_12_2.zip
- fluent-bit-cmake-topic-pal-strerror_r-wrapper-github-pr-debian_x86_64_12_2.log
  The cmake log.
- fluent-bit-make-topic-pal-strerror_r-wrapper-github-pr-debian_x86_64_12_2.log
  The make log.
- fluent-bit-ctest-topic-pal-strerror_r-wrapper-github-pr-debian_x86_64_12_2.log
  The ctest log.
- fluent-bit-ctest-topic-pal-strerror_r-wrapper-github-pr-debian_x86_64_12_2-LastTest.log
  The LastTest.log file out of ctest.
- fluent-bit-valgrind-topic-pal-strerror_r-wrapper-github-pr-debian_x86_64_12_2.log
  The valgrind --leak-check=full log.
  No memory leak happened.
- gcov-tests-topic-pal-strerror_r-wrapper-github-pr-debian_x86_64_12_2.tar.xz
  The lcov coverage report.
  - The new code has been covered except for one.
    - Can we depend on the glibc strerror_r(3) never returning NULL?
FreeBSD/amd64 14.0-RELEASE: logs-fluent-bit-topic-pal-strerror_r-wrapper-freebsd_amd64_14_0_release.zip
- fluent-bit-cmake-topic-pal-strerror_r-wrapper-freebsd_amd64_14_0_release.log
  The cmake log.
- fluent-bit-make-topic-pal-strerror_r-wrapper-freebsd_amd64_14_0_release.log
  The make log.
- fluent-bit-ctest-topic-pal-strerror_r-wrapper-freebsd_amd64_14_0_release.log
  The ctest log.
- fluent-bit-ctest-topic-pal-strerror_r-wrapper-freebsd_amd64_14_0_release-LastTest.log
  The LastTest.log file out of ctest.
- fluent-bit-valgrind-topic-pal-strerror_r-wrapper-freebsd_amd64_14_0_release.log
  The valgrind --leak-check=full log.
  - The coverage is disabled because of the Valgrind problem that makes an assertion failure.
  - Many conditional jumps or moves on uninitialised value.
    - These are the false errors; no conditional instructions have been found out of the codewalk.
  - 3 blocks are still reachable with flb_strerror_r().
    - These are for the NLS message catalogue, read only once and staying until the process exits.
  - The rest of the leaks do not involve the Fluent Bit implementation.
- llvm-cov-tests-topic-pal-strerror_r-wrapper-freebsd_amd64_14_0_release.tar.xz
  The LLVM coverage report.
  - The new code has been covered.

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

N/A Run local packaging test showing all targets (including any new ones) build.
N/A Set ok-package-test label to test for all targets (requires maintainer to do).
Packaging is not affected.

Documentation

? Documentation required for this feature
The pal paragraph in DEVELOPER_GUIDE.md?

Backporting

Backport to latest stable release.
This PR is the fix to a build breakage.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Glibc has the different return type and return value semantics from POSIX.1-2001 on strerror_r(3). Also, the glibc strerror_r(3) may return the pointer different from the given one. The difference will be absorbed by the platform abstraction layer, committed separately. Signed-off-by: Seigo Tanimura <[email protected]>

The signature is that of POSIX.1-2001. If the returned string is not in the supplied buffer, copy the string to it. Signed-off-by: Seigo Tanimura <[email protected]>

Actually, only the external behaviour is tested; there is no support for the mock of the library functions in general. Signed-off-by: Seigo Tanimura <[email protected]>

flb_strerror_r() always terminates the string buffer by NUL. Signed-off-by: Seigo Tanimura <[email protected]>

flb_strerror_r() always returns the string into the supplied buffer. Signed-off-by: Seigo Tanimura <[email protected]>

It is the C11 edition of strerror_r(3), provided by MSVC. Signed-off-by: Seigo Tanimura <[email protected]>

Signed-off-by: Seigo Tanimura <[email protected]>

…r(3). It is misleading that the return type of strerror_s(3) is errno_t, while its semantics is the int a la the Unix system calls. While I am here, fix the unintended tabs. Signed-off-by: Seigo Tanimura <[email protected]>

Such the condition seems not treated as a runtime constraint violation. The best solution is hence to guarantee that the zero status means that a NUL-terminated string is returned. Signed-off-by: Seigo Tanimura <[email protected]>

Strerror_s(3) does not detect the short buffer, and that cannot be covered by pal. Signed-off-by: Seigo Tanimura <[email protected]>

Signed-off-by: Seigo Tanimura <[email protected]>

github-actions · 2024-04-19T01:49:31Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2024-12-14T02:07:44Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

altimeter-130ft added 6 commits January 18, 2024 14:52

pal: Add flb_strerror_r(), the wrapper of strerror_r(3).

0894a12

The signature is that of POSIX.1-2001. If the returned string is not in the supplied buffer, copy the string to it. Signed-off-by: Seigo Tanimura <[email protected]>

tests: Add the internal unit test for the pal module.

3bc211b

Actually, only the external behaviour is tested; there is no support for the mock of the library functions in general. Signed-off-by: Seigo Tanimura <[email protected]>

io: Replace strerror_r(3) by flb_strerror_r().

adb523f

flb_strerror_r() always terminates the string buffer by NUL. Signed-off-by: Seigo Tanimura <[email protected]>

log: Replace strerror_r(3) by flb_strerror_r().

449bf34

flb_strerror_r() always terminates the string buffer by NUL. Signed-off-by: Seigo Tanimura <[email protected]>

network: Replace strerror_r(3) by flb_strerror_r().

18fb467

flb_strerror_r() always returns the string into the supplied buffer. Signed-off-by: Seigo Tanimura <[email protected]>

github-actions bot added the docs-required label Jan 18, 2024

altimeter-130ft mentioned this pull request Jan 18, 2024

cmake: add LLVM coverage support #8376

Open

3 tasks

altimeter-130ft marked this pull request as ready for review January 18, 2024 15:29

altimeter-130ft requested review from fujimotos, niedbalski, patrick-stephens, celalettin1286, edsiper, leonardo-albertovich and koleini as code owners January 18, 2024 15:29

altimeter-130ft mentioned this pull request Jan 18, 2024

Remove glibc dependency on and misuse of strerror_r(3) #8379

Closed

altimeter-130ft added 6 commits January 19, 2024 01:19

cmake: Add the check of strerror_s(3).

0b4de13

It is the C11 edition of strerror_r(3), provided by MSVC. Signed-off-by: Seigo Tanimura <[email protected]>

pal: Add the strerror_s(3) implementation to flb_strerror_r().

76df948

Signed-off-by: Seigo Tanimura <[email protected]>

tests: Test flb_strerror_r() when strerror_s(3) is found.

cec87d7

Signed-off-by: Seigo Tanimura <[email protected]>

tests: Work around the strerror_s(3) limitation.

0de702a

Strerror_s(3) does not detect the short buffer, and that cannot be covered by pal. Signed-off-by: Seigo Tanimura <[email protected]>

patrick-stephens added the ok-package-test Run PR packaging tests label Jan 19, 2024

altimeter-130ft temporarily deployed to pr January 19, 2024 09:14 — with GitHub Actions Inactive

altimeter-130ft temporarily deployed to pr January 19, 2024 09:35 — with GitHub Actions Inactive

cmake: Avoid the false detection of glibc strerror_r(3) on MacOS.

8dd4511

Signed-off-by: Seigo Tanimura <[email protected]>

altimeter-130ft temporarily deployed to pr January 19, 2024 11:31 — with GitHub Actions Inactive

altimeter-130ft temporarily deployed to pr January 19, 2024 11:51 — with GitHub Actions Inactive

github-actions bot added the Stale label Apr 19, 2024

github-actions bot removed the Stale label Aug 16, 2024

github-actions bot added the Stale label Dec 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pal: platform abstraction layer and strerror_r(3) wrapper #8390

pal: platform abstraction layer and strerror_r(3) wrapper #8390

altimeter-130ft commented Jan 18, 2024

github-actions bot commented Apr 19, 2024

github-actions bot commented Dec 14, 2024

pal: platform abstraction layer and strerror_r(3) wrapper #8390

Are you sure you want to change the base?

pal: platform abstraction layer and strerror_r(3) wrapper #8390

Conversation

altimeter-130ft commented Jan 18, 2024

Fixed Issue

Background

Changes

pal Source Files

pal Internal Test Files

New pal Function

flb_strerror_r() Callers

Tested Environments

Testing

Documentation

Backporting

github-actions bot commented Apr 19, 2024

github-actions bot commented Dec 14, 2024

`pal` Source Files

`pal` Internal Test Files

New `pal` Function

`flb_strerror_r()` Callers