Attention is currently required from: Anastasia Klimchuk, Anton Samsonov, Anton Samsonov.

View Change

3 comments:

• Patchset:

  • Patch Set #1:

    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:

  • Patch Set #1, 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

  • Patch Set #1, 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?

To view, visit change 77089. To unsubscribe, or for help writing mail filters, visit settings.