On 04.11.21 10:24, Michael Adler wrote:
> Below is the delta to the v2 patch series.
> Apart from that, a few commit messages have been improved, as suggested in
> the v2 review.
>
> diff --git a/.clang-format b/.clang-format
> index 49dee99..420c4ff 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -16,3 +16,7 @@ SortIncludes: false
> UseTab: Always
> ContinuationIndentWidth: 8
> AlignConsecutiveMacros: true
> +# Typical macros are expressions, and require a semi-colon to be added;
> sometimes this is not the case, and this allows
> +# to make clang-format aware of such cases.
> +StatementMacros:
> + - OPT
> diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
> index 4b9884f..7a234cf 100644
> --- a/tools/bg_setenv.c
> +++ b/tools/bg_setenv.c
> @@ -25,46 +25,52 @@
> static char doc[] =
> "bg_setenv/bg_printenv - Environment tool for the EFI Boot Guard";
>
> -// clang-format off
> +#define OPT(name, key, arg, flags, doc)
> \
> + { \
> + name, key, arg, flags, doc \
> + }
> +
> #define BG_CLI_OPTIONS_COMMON
> \
> - {"filepath", 'f', "ENVFILE", 0, \
> - "Environment to use. Expects a file name, " \
> - "usually called BGENV.DAT."}, \
> - {"part", 'p', "ENV_PART", 0, \
> - "Set environment partition to use. If no partition is specified, " \
> - "the one with the smallest revision value above zero is used."}, \
> - {"verbose", 'v', 0, 0, "Be verbose"}, \
> - {"version", 'V', 0, 0, "Print version"}
> -// clang-format on
> + OPT("filepath", 'f', "ENVFILE", 0, \
> + "Environment to use. Expects a file name, " \
> + "usually called BGENV.DAT.") \
> + , OPT("part", 'p', "ENV_PART", 0, \
> + "Set environment partition to update. If no partition is " \
> + "specified, " \
> + "the one with the smallest revision value above zero is " \
> + "updated.") \
> + , OPT("verbose", 'v', 0, 0, "Be verbose") \
> + , OPT("version", 'V', 0, 0, "Print version")
>
> static struct argp_option options_setenv[] = {
> BG_CLI_OPTIONS_COMMON,
> - {"preserve", 'P', 0, 0, "Preserve existing entries"},
> - {"kernel", 'k', "KERNEL", 0, "Set kernel to load"},
> - {"args", 'a', "KERNEL_ARGS", 0, "Set kernel arguments"},
> - {"revision", 'r', "REVISION", 0, "Set revision value"},
> - {"ustate", 's', "USTATE", 0, "Set update status for environment"},
> - {"watchdog", 'w', "WATCHDOG_TIMEOUT", 0, "Watchdog timeout in seconds"},
> - {"confirm", 'c', 0, 0, "Confirm working environment"},
> - {"update", 'u', 0, 0, "Automatically update oldest revision"},
> - {"uservar", 'x', "KEY=VAL", 0,
> - "Set user-defined string variable. For setting multiple variables, "
> - "use this option multiple times."},
> - {"in_progress", 'i', "IN_PROGRESS", 0,
> - "Set in_progress variable to simulate a running update process."},
> + OPT("preserve", 'P', 0, 0, "Preserve existing entries"),
> + OPT("kernel", 'k', "KERNEL", 0, "Set kernel to load"),
> + OPT("args", 'a', "KERNEL_ARGS", 0, "Set kernel arguments"),
> + OPT("revision", 'r', "REVISION", 0, "Set revision value"),
> + OPT("ustate", 's', "USTATE", 0, "Set update status for environment"),
> + OPT("watchdog", 'w', "WATCHDOG_TIMEOUT", 0,
> + "Watchdog timeout in seconds"),
> + OPT("confirm", 'c', 0, 0, "Confirm working environment"),
> + OPT("update", 'u', 0, 0, "Automatically update oldest revision"),
> + OPT("uservar", 'x', "KEY=VAL", 0,
> + "Set user-defined string variable. For setting multiple variables, "
> + "use this option multiple times."),
> + OPT("in_progress", 'i', "IN_PROGRESS", 0,
> + "Set in_progress variable to simulate a running update process."),
> {},
> };
>
> static struct argp_option options_printenv[] = {
> BG_CLI_OPTIONS_COMMON,
> - {"current", 'c', 0, 0,
> - "Only print values from the current environment"},
> - {"output", 'o', "LIST", 0,
> - "Comma-separated list of fields which are printed. "
> - "Available fields: in_progress, revision, kernel, kernelargs, "
> - "watchdog_timeout, ustate, user. "
> - "If omitted, all available fields are printed."},
> - {"raw", 'r', 0, 0, "Raw output mode, e.g. for shell scripting"},
> + OPT("current", 'c', 0, 0,
> + "Only print values from the current environment"),
> + OPT("output", 'o', "LIST", 0,
> + "Comma-separated list of fields which are printed. "
> + "Available fields: in_progress, revision, kernel, kernelargs, "
> + "watchdog_timeout, ustate, user. "
> + "If omitted, all available fields are printed."),
> + OPT("raw", 'r', 0, 0, "Raw output mode, e.g. for shell scripting"),
> {},
> };
>
> @@ -82,32 +88,34 @@ struct arguments_common {
> struct arguments_setenv {
> struct arguments_common common;
> /* auto update feature automatically updates partition with
> - * oldest environment revision (lowest value)
> - */
> + * oldest environment revision (lowest value) */
> bool auto_update;
> /* whether to keep existing entries in BGENV before applying new
> * settings */
> bool preserve_env;
> };
>
> +struct fields {
> + unsigned int in_progress : 1;
> + unsigned int revision : 1;
> + unsigned int kernel : 1;
> + unsigned int kernelargs : 1;
> + unsigned int wdog_timeout : 1;
> + unsigned int ustate : 1;
> + unsigned int user : 1;
> +};
> +
> +static const struct fields ALL_FIELDS = {1, 1, 1, 1, 1, 1, 1};
> +
> /* Arguments used by bg_printenv. */
> struct arguments_printenv {
> struct arguments_common common;
> bool current;
> /* a bitset to decide which fields are printed */
> - int output_fields;
> + struct fields output_fields;
> bool raw;
> };
>
> -#define MASK_IN_PROGRESS (1 << 0)
> -#define MASK_REVISION (1 << 1)
> -#define MASK_KERNEL (1 << 2)
> -#define MASK_KERNELARGS (1 << 3)
> -#define MASK_WDOG_TIMEOUT (1 << 4)
> -#define MASK_USTATE (1 << 5)
> -#define MASK_USER (1 << 6)
> -#define MASK_ALL 0x7F /* binary: 1111111 */
> -
> typedef enum { ENV_TASK_SET, ENV_TASK_DEL } BGENV_TASK;
>
> struct stailhead *headp;
> @@ -478,27 +486,26 @@ static error_t parse_setenv_opt(int key, char *arg,
> struct argp_state *state)
> return e;
> }
>
> -static error_t parse_output_fields(char *fields, int *output_fields)
> +static error_t parse_output_fields(char *fields, struct fields
> *output_fields)
> {
> char *token;
> - /* reset */
> - *output_fields = 0;
> + memset(output_fields, 0, sizeof(struct fields));
> while ((token = strsep(&fields, ","))) {
> if (*token == '\0') continue;
> if (strcmp(token, "in_progress") == 0) {
> - *output_fields |= MASK_IN_PROGRESS;
> + output_fields->in_progress = true;
> } else if (strcmp(token, "revision") == 0) {
> - *output_fields |= MASK_REVISION;
> + output_fields->revision = true;
> } else if (strcmp(token, "kernel") == 0) {
> - *output_fields |= MASK_KERNEL;
> + output_fields->kernel = true;
> } else if (strcmp(token, "kernelargs") == 0) {
> - *output_fields |= MASK_KERNELARGS;
> + output_fields->kernelargs = true;
> } else if (strcmp(token, "watchdog_timeout") == 0) {
> - *output_fields |= MASK_WDOG_TIMEOUT;
> + output_fields->wdog_timeout = true;
> } else if (strcmp(token, "ustate") == 0) {
> - *output_fields |= MASK_USTATE;
> + output_fields->ustate = true;
> } else if (strcmp(token, "user") == 0) {
> - *output_fields |= MASK_USER;
> + output_fields->user = true;
> } else {
> fprintf(stderr, "Unknown output field: %s\n", token);
> return 1;
> @@ -604,13 +611,13 @@ static void dump_uservars(uint8_t *udata, bool raw)
> }
> }
>
> -static void dump_env(BG_ENVDATA *env, int output_fields, bool raw)
> +static void dump_env(BG_ENVDATA *env, struct fields output_fields, bool raw)
> {
> char buffer[ENV_STRING_LENGTH];
> if (!raw) {
> fprintf(stdout, "Values:\n");
> }
> - if (output_fields & MASK_IN_PROGRESS) {
> + if (output_fields.in_progress) {
> if (raw) {
> fprintf(stdout, "IN_PROGRESS=%d\n", env->in_progress);
> } else {
> @@ -618,7 +625,7 @@ static void dump_env(BG_ENVDATA *env, int output_fields,
> bool raw)
> env->in_progress ? "yes" : "no");
> }
> }
> - if (output_fields & MASK_REVISION) {
> + if (output_fields.revision) {
> if (raw) {
> fprintf(stdout, "REVISION=%d\n", env->revision);
> } else {
> @@ -626,7 +633,7 @@ static void dump_env(BG_ENVDATA *env, int output_fields,
> bool raw)
> env->revision);
> }
> }
> - if (output_fields & MASK_KERNEL) {
> + if (output_fields.kernel) {
> char *kernelfile = str16to8(buffer, env->kernelfile);
> if (raw) {
> fprintf(stdout, "KERNEL=%s\n", kernelfile);
> @@ -634,7 +641,7 @@ static void dump_env(BG_ENVDATA *env, int output_fields,
> bool raw)
> fprintf(stdout, "kernel: %s\n", kernelfile);
> }
> }
> - if (output_fields & MASK_KERNELARGS) {
> + if (output_fields.kernelargs) {
> char *kernelargs = str16to8(buffer, env->kernelparams);
> if (raw) {
> fprintf(stdout, "KERNELARGS=%s\n", kernelargs);
> @@ -642,7 +649,7 @@ static void dump_env(BG_ENVDATA *env, int output_fields,
> bool raw)
> fprintf(stdout, "kernelargs: %s\n", kernelargs);
> }
> }
> - if (output_fields & MASK_WDOG_TIMEOUT) {
> + if (output_fields.wdog_timeout) {
> if (raw) {
> fprintf(stdout, "WATCHDOG_TIMEOUT=%u\n",
> env->watchdog_timeout_sec);
> @@ -651,7 +658,7 @@ static void dump_env(BG_ENVDATA *env, int output_fields,
> bool raw)
> env->watchdog_timeout_sec);
> }
> }
> - if (output_fields & MASK_USTATE) {
> + if (output_fields.ustate) {
> if (raw) {
> fprintf(stdout, "USTATE=%u\n", env->ustate);
> } else {
> @@ -659,7 +666,7 @@ static void dump_env(BG_ENVDATA *env, int output_fields,
> bool raw)
> (uint8_t)env->ustate, ustate2str(env->ustate));
> }
> }
> - if (output_fields & MASK_USER) {
> + if (output_fields.user) {
> if (!raw) {
> fprintf(stdout, "\n");
> fprintf(stdout, "user variables:\n");
> @@ -690,7 +697,7 @@ static void update_environment(BGENV *env, bool verbosity)
>
> }
>
> -static void dump_envs(int output_fields, bool raw)
> +static void dump_envs(struct fields output_fields, bool raw)
> {
> for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
> if (!raw) {
> @@ -709,7 +716,7 @@ static void dump_envs(int output_fields, bool raw)
> }
> }
>
> -static void dump_latest_env(int output_fields, bool raw)
> +static void dump_latest_env(struct fields output_fields, bool raw)
> {
> BGENV *env = bgenv_open_latest();
> if (!env) {
> @@ -720,7 +727,8 @@ static void dump_latest_env(int output_fields, bool raw)
> bgenv_close(env);
> }
>
> -static void dump_env_by_index(uint32_t index, int output_fields, bool raw)
> +static void dump_env_by_index(uint32_t index, struct fields output_fields,
> + bool raw)
> {
> BGENV *env = bgenv_open_by_index(index);
> if (!env) {
> @@ -756,7 +764,8 @@ static bool get_env(char *configfilepath, BG_ENVDATA
> *data)
> return result;
> }
>
> -static int printenv_from_file(char *envfilepath, int output_fields, bool raw)
> +static int printenv_from_file(char *envfilepath, struct fields output_fields,
> + bool raw)
> {
> int success = 0;
> BG_ENVDATA data;
> @@ -788,7 +797,7 @@ static int dumpenv_to_file(char *envfilepath, bool
> verbosity, bool preserve_env)
>
> update_environment(&env, verbosity);
> if (verbosity) {
> - dump_env(env.data, MASK_ALL, false);
> + dump_env(env.data, ALL_FIELDS, false);
> }
> FILE *of = fopen(envfilepath, "wb");
> if (of) {
> @@ -823,7 +832,7 @@ static error_t bg_printenv(int argc, char **argv)
> };
>
> struct arguments_printenv arguments = {
> - .output_fields = MASK_ALL,
> + .output_fields = ALL_FIELDS,
> };
>
> error_t e = argp_parse(&argp_printenv, argc, argv, 0, 0, &arguments);
> @@ -940,7 +949,7 @@ static error_t bg_setenv(int argc, char **argv)
> }
>
> if (arguments.common.verbosity) {
> - dump_envs(MASK_ALL, false);
> + dump_envs(ALL_FIELDS, false);
> }
>
> BGENV *env_new = NULL;
> @@ -1004,7 +1013,7 @@ static error_t bg_setenv(int argc, char **argv)
> if (arguments.common.verbosity) {
> fprintf(stdout, "New environment data:\n");
> fprintf(stdout, "---------------------\n");
> - dump_env(env_new->data, MASK_ALL, false);
> + dump_env(env_new->data, ALL_FIELDS, false);
> }
> if (!bgenv_write(env_new)) {
> fprintf(stderr, "Error storing environment.\n");
>
>
Thanks, applied.
Jan
--
You received this message because you are subscribed to the Google Groups "EFI
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/efibootguard-dev/7045e375-ae7c-f371-972e-d9b6defb24a5%40web.de.
