From f9e3084dafadf7f14ceae871b7fc18b457b68df2 Mon Sep 17 00:00:00 2001 From: Alberto Escolar Piedras Date: Fri, 16 Feb 2024 16:52:56 +0100 Subject: [PATCH] scripts/checkpatch: Limit which feature test macros we prevent Unfortunately this check as it is today is causing trouble, while not checking too well for what it intended. Let's reduce its scope until a better solution has been found. Background: This check intends to ensure coding guidelines Rules A.4 and A.5 are followed, but how it is implemented it does not work well enough. 1. These rules only apply to the kernel and some other parts of the embedded codebase respectively, but this check is performed on the whole tree. 2. This check works under the assumption that any attempt to set these macros in source files is a violation of these rules, while this is not necessary the case, as there are legitimate uses for these. (Specially for _POSIX_C_SOURCE and _XOPEN_SOURCE) This check also fails to detect these macros being set in cmake files, so if users are faced with this failure they can trivially bypass it. Having a CI check which produces too many false positives, while at the same time being very easy to bypass is not a desirable situation as that can result in lack of trust for this type of checks, and an overall tendency to override these CI faults, and overlooking actual violations of these rules by reviewers. This check was originally added in b021dece987d691342636dad836d6c9611d510e1 Signed-off-by: Alberto Escolar Piedras --- scripts/checkpatch.pl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index b3bfd180cca..833b11886fa 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -600,10 +600,8 @@ our $api_defines = qr{(?x: _GNU_SOURCE| _ISOC11_SOURCE| _ISOC99_SOURCE| - _POSIX_C_SOURCE| _POSIX_SOURCE| _SVID_SOURCE| - _XOPEN_SOURCE| _XOPEN_SOURCE_EXTENDED )}; @@ -6550,7 +6548,7 @@ sub process { if ($line =~ /#\s*define\s+$api_defines/) { ERROR("API_DEFINE", - "do not specify a non-Zephyr API for libc\n" . "$here$rawline\n"); + "do not specify non-standard feature test macros for embedded code\n" . "$here$rawline\n"); } # check for IS_ENABLED() without CONFIG_ ($rawline for comments too)