[PATCH 15/18] cmd: log: Add commands to manipulate filters

[Heinrich Schuchardt]

On 10/6/20 9:16 PM, Sean Anderson wrote:
> This adds several commands to add, list, and remove log filters. Due to the
> complexity of adding a filter, `log filter-list` uses options instead of
> positional arguments.
>
> These commands have been added as subcommands to log by using a dash to
> join the subcommand and subsubcommand. This is stylistic, and they could be
> converted to proper subsubcommands if it is wished.
>
> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> ---
>
>  cmd/Kconfig |   1 +
>  cmd/log.c   | 213 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 214 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 999b6cf239..c5cb112171 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2255,6 +2255,7 @@ config CMD_KGDB
>  config CMD_LOG
>  	bool "log - Generation, control and access to logging"
>  	select LOG
> +	select GETOPT
>  	help
>  	  This provides access to logging features. It allows the output of
>  	  log data to be controlled to a limited extent (setting up the default
> diff --git a/cmd/log.c b/cmd/log.c
> index 71bfdbbc0f..c9eaff7b94 100644
> --- a/cmd/log.c
> +++ b/cmd/log.c
> @@ -7,6 +7,7 @@
>  #include <common.h>
>  #include <command.h>
>  #include <dm.h>
> +#include <getopt.h>
>  #include <log.h>
>
>  static char log_fmt_chars[LOGF_COUNT] = "clFLfm";
> @@ -43,6 +44,195 @@ static int do_log_level(struct cmd_tbl *cmdtp, int flag, int argc,
>  	return CMD_RET_SUCCESS;
>  }
>
> +static int do_log_filter_list(struct cmd_tbl *cmdtp, int flag, int argc,
> +			      char *const argv[])
> +{
> +	int opt;
> +	const char *drv_name = "console";
> +	struct getopt_state gs;
> +	struct log_filter *filt;
> +	struct log_device *ldev;
> +
> +	getopt_init_state(&gs);
> +	while ((opt = getopt(&gs, argc, argv, "d:")) > 0) {
> +		switch (opt) {
> +		case 'd':
> +			drv_name = gs.optarg;
> +			break;
> +		default:
> +			return CMD_RET_USAGE;
> +		}
> +	}
> +
> +	if (gs.optind != argc)
> +		return CMD_RET_USAGE;
> +
> +	ldev = log_device_find_by_name(drv_name);
> +	if (!ldev) {
> +		printf("Could not find log device for \"%s\"\n", drv_name);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	printf("num policy level           categories files\n");
> +	list_for_each_entry(filt, &ldev->filter_head, sibling_node) {
> +		printf("%3d %6.6s %s%-7.7s ", filt->filter_num,
> +		       filt->flags & LOGFF_DENY ? "deny" : "allow",
> +		       filt->flags & LOGFF_LEVEL_MIN ? ">=" : "<=",
> +		       log_get_level_name(filt->level));
> +
> +		if (filt->flags & LOGFF_HAS_CAT) {
> +			int i;
> +
> +			if (filt->cat_list[0] != LOGC_END)
> +				printf("%16.16s %s\n",
> +				       log_get_cat_name(filt->cat_list[0]),
> +				       filt->file_list ? filt->file_list : "");
> +
> +			for (i = 1; i < LOGF_MAX_CATEGORIES &&
> +				    filt->cat_list[i] != LOGC_END; i++)
> +				printf("%20c %16.16s\n", ' ',
> +				       log_get_cat_name(filt->cat_list[i]));
> +		} else {
> +			printf("%16c %s\n", ' ',
> +			       filt->file_list ? filt->file_list : "");
> +		}
> +	}
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
> +static int do_log_filter_add(struct cmd_tbl *cmdtp, int flag, int argc,
> +			     char *const argv[])
> +{
> +	bool level_set = false;
> +	bool print_num = false;
> +	bool type_set = false;
> +	char *file_list = NULL;
> +	const char *drv_name = "console";
> +	int opt, err;
> +	int cat_count = 0;
> +	int flags = 0;
> +	enum log_category_t cat_list[LOGF_MAX_CATEGORIES + 1];
> +	enum log_level_t level = LOGL_MAX;
> +	struct getopt_state gs;
> +
> +	getopt_init_state(&gs);
> +	while ((opt = getopt(&gs, argc, argv, "Ac:d:Df:l:L:p")) > 0) {
> +		switch (opt) {
> +		case 'A':
> +#define do_type() do { \
> +			if (type_set) { \
> +				printf("Allow or deny set twice\n"); \
> +				return CMD_RET_USAGE; \
> +			} \
> +			type_set = true; \
> +} while (0)
> +			do_type();
> +			break;
> +		case 'c': {
> +			enum log_category_t cat;
> +
> +			if (cat_count >= LOGF_MAX_CATEGORIES) {
> +				printf("Too many categories\n");
> +				return CMD_RET_FAILURE;
> +			}
> +
> +			cat = log_get_cat_by_name(gs.optarg);
> +			if (cat == LOGC_NONE) {
> +				printf("Unknown category \"%s\"\n", gs.optarg);
> +				return CMD_RET_FAILURE;
> +			}
> +
> +			cat_list[cat_count++] = cat;
> +			break;
> +		}
> +		case 'd':
> +			drv_name = gs.optarg;
> +			break;
> +		case 'D':
> +			do_type();
> +			flags |= LOGFF_DENY;
> +			break;
> +		case 'f':
> +			file_list = gs.optarg;
> +			break;
> +		case 'l':
> +#define do_level() do { \
> +			if (level_set) { \
> +				printf("Log level set twice\n"); \
> +				return CMD_RET_USAGE; \
> +			} \
> +			level = parse_log_level(gs.optarg); \
> +			if (level == LOGL_NONE) \
> +				return CMD_RET_FAILURE; \
> +			level_set = true; \
> +} while (0)
> +			do_level();
> +			break;
> +		case 'L':
> +			do_level();
> +			flags |= LOGFF_LEVEL_MIN;
> +			break;
> +		case 'p':
> +			print_num = true;
> +			break;
> +		default:
> +			return CMD_RET_USAGE;
> +		}
> +	}
> +
> +	if (gs.optind != argc)
> +		return CMD_RET_USAGE;
> +
> +	cat_list[cat_count] = LOGC_END;
> +	err = log_add_filter_flags(drv_name, cat_count ? cat_list : NULL, level,
> +				   file_list, flags);
> +	if (err < 0) {
> +		printf("Could not add filter (err = %d)\n", err);
> +		return CMD_RET_FAILURE;
> +	} else if (print_num) {
> +		printf("%d\n", err);
> +	}
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
> +static int do_log_filter_remove(struct cmd_tbl *cmdtp, int flag, int argc,
> +				char *const argv[])
> +{
> +	int opt, err;
> +	ulong filter_num;
> +	const char *drv_name = "console";
> +	struct getopt_state gs;
> +
> +	getopt_init_state(&gs);
> +	while ((opt = getopt(&gs, argc, argv, "d:")) > 0) {
> +		switch (opt) {
> +		case 'd':
> +			drv_name = gs.optarg;
> +			break;
> +		default:
> +			return CMD_RET_USAGE;
> +		}
> +	}
> +
> +	if (gs.optind + 1 != argc)
> +		return CMD_RET_USAGE;
> +
> +	if (strict_strtoul(argv[gs.optind], 10, &filter_num)) {
> +		printf("Invalid filter number \"%s\"\n", argv[gs.optind]);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	err = log_remove_filter(drv_name, filter_num);
> +	if (err) {
> +		printf("Could not remove filter (err = %d)\n", err);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
>  static int do_log_format(struct cmd_tbl *cmdtp, int flag, int argc,
>  			 char *const argv[])
>  {
> @@ -122,6 +312,25 @@ static char log_help_text[] =
>  #if CONFIG_IS_ENABLED(LOG_TEST)
>  	"log test - run log tests\n"

This is probably what a typical user is least interested in. Please,
move it to the end of the description.

>  #endif
> +	"log filter-list [OPTIONS] - list all filters for a log driver\n"
> +	"\t-d <driver> - Specify the log driver to add the filter to; defaults\n"> +	"\t              to console\n"

The text seems not to relate to filter-list.

Why don't we filter all log drivers by default? When would a user be
interested to filter by driver?

How should I know which drivers and categories exist on my system?

> +	"log filter-add [OPTIONS] - add a new filter to a driver\n"
> +	"\t-A - Allow messages matching this filter; mutually exclusive with -D\n"
> +	"\t     This is the default.\n"
> +	"\t-c <category> - Category to match; may be specified multiple times\n"
> +	"\t-d <driver> - Specify the log driver to add the filter to; defaults\n"
> +	"\t              to console\n"
> +	"\t-D - Deny messages matching this filter; mutually exclusive with -A\n"
> +	"\t-f <files_list> - A comma-separated list of files to match\n"
> +	"\t-l <level> - Match log levels below this level; mutually exclusive\n"
> +	"\t             with -L\n"

Is this below or below and equal?

> +	"\t-L <level> - Match log levels above this level; mutually exclusive\n"
> +	"\t             with -l\n"

Is this above or above and equal?

> +	"\t-p - Print the filter number on success\n"

Why do we need this parameter? Can't we simply always print the filter
numbers or never?

> +	"log filter-remove [OPTIONS] <num> - Remove filter number <num>\n"
> +	"\t-d <driver> - Specify the log driver to add the filter to; defaults\n"
> +	"\t              to console\n"

This entry seems not to be related to removal.

How do I simply delete all filters?
How do I remove all filters for a category?

Best regards

Heinrich

>  	"log format <fmt> - set log output format. <fmt> is a string where\n"
>  	"\teach letter indicates something that should be displayed:\n"
>  	"\tc=category, l=level, F=file, L=line number, f=function, m=msg\n"
> @@ -136,6 +345,10 @@ U_BOOT_CMD_WITH_SUBCMDS(log, "log system", log_help_text,
>  #if CONFIG_IS_ENABLED(LOG_TEST)
>  	U_BOOT_SUBCMD_MKENT(test, 2, 1, do_log_test),
>  #endif
> +	U_BOOT_SUBCMD_MKENT(filter-list, 3, 1, do_log_filter_list),
> +	U_BOOT_SUBCMD_MKENT(filter-add, CONFIG_SYS_MAXARGS, 1,
> +			    do_log_filter_add),
> +	U_BOOT_SUBCMD_MKENT(filter-remove, 4, 1, do_log_filter_remove),
>  	U_BOOT_SUBCMD_MKENT(format, 2, 1, do_log_format),
>  	U_BOOT_SUBCMD_MKENT(rec, 7, 1, do_log_rec),
>  );
>