Attention is currently required from: Anastasia Klimchuk, Anton Samsonov, Anton Samsonov.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email )
Change subject: Remove dependency on C23 __has_include() ......................................................................
Patch Set 1:
(3 comments)
Patchset:
PS1:
In my understanding, this patch may be needed for all compilers based on legacy versions of EDG fron […]
Appreciate your explanation. I don't think there's any point in describing it in the commit message. However, if you include something like "This patch has been tested on EDG 4.14 / GCC 5.5" (using the version you tested on, of course) it would make it easier for anyone who wants to test this patch to reproduce it
File include/cli_classic.h:
https://review.coreboot.org/c/flashrom/+/77089/comment/a283f041_ff4d2e23 : PS1, Line 21: #if !defined(__has_include) && (__STDC_VERSION__ < 202300L) : #include <getopt.h> : #define HAVE_GETOPT_H 1 If we cannot identify whether this header file exists on the user's system, then let's assume it doesn't. So I suggest we drop this branch
https://review.coreboot.org/c/flashrom/+/77089/comment/adc99f67_4fb99312 : PS1, Line 18: #if HAVE_GETOPT_H : #include <getopt.h> : #elif !defined(HAVE_GETOPT_H) : #if !defined(__has_include) && (__STDC_VERSION__ < 202300L) : #include <getopt.h> : #define HAVE_GETOPT_H 1 : #elif __has_include(<getopt.h>) : #include <getopt.h> : #define HAVE_GETOPT_H 1 : #else : #define HAVE_GETOPT_H 0 : #endif /* __has_include() */ : #endif /* HAVE_GETOPT_H */ : : #if !HAVE_GETOPT_H Our and coreboot code styles say nothing about directives. I'm having trouble reading if's branches without nesting. I would suggest to use indent after `#`, at least the coding style does not forbid it (I hope so).
What do you think, Anastasia?
-
Alexander Goncharov (Code Review)