[PATCH v2 00/22] log: Add commands for manipulating filters

classic Classic list List threaded Threaded
43 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 00/22] log: Add commands for manipulating filters

Sean Anderson
This series adds several commands for adding, listing, and removing log filters.
It also adds getopt, since the filter-add command needs to have several
optional arguments to be complete, and positional specification of those
arguments would have been difficult.

Changes in v2:
- Add % before constants in kerneldocs
- Add compiletime assert on size of log_cat_name
- Add const qualifier to log_*_name
- Add option to remove all filters to filter-remove
- Clarify filter-* help text
- Clarify wording of filter documentation
- Converted log filter-* tests to C from python
- Document log_level_t and log_category_t members
- Expand documentation of getopt() to include examples
- Expose log_has_cat and log_has_file for filter tests
- Include enum definitions instead of re-documenting them
- Print an error message if the log level is invalid.
- Remove opt prefix from getopt_state members
- Reorganize log documentation; related sections should now be more proximate

Sean Anderson (22):
  log: Fix missing negation of ENOMEM
  log: Fix incorrect documentation of log_filter.cat_list
  log: Add additional const qualifier to arrays
  log: Add new category names to log_cat_name
  log: Use CONFIG_IS_ENABLED() for LOG_TEST
  log: Expose some helper functions
  log: Add function to create a filter with flags
  log: Add filter flag to deny on match
  test: Add tests for LOGFF_DENY
  log: Add filter flag to match greater than a log level
  test: Add test for LOGFF_MIN
  cmd: log: Use sub-commands for log
  cmd: log: Split off log level parsing
  cmd: log: Move log test to end of help string
  cmd: log: Add commands to list categories and drivers
  cmd: log: Make "log level" print all log levels
  lib: Add getopt
  test: Add a test for getopt
  cmd: log: Add commands to manipulate filters
  test: Add a test for log filter-*
  doc: Add log kerneldocs to documentation
  doc: Update logging documentation

 MAINTAINERS               |   1 +
 cmd/Kconfig               |   1 +
 cmd/log.c                 | 360 +++++++++++++++++++++++++++++++++-----
 common/log.c              |  65 ++++---
 doc/api/getopt.rst        |   8 +
 doc/api/index.rst         |   1 +
 doc/develop/logging.rst   | 238 ++++++++++++-------------
 include/getopt.h          | 130 ++++++++++++++
 include/log.h             | 205 ++++++++++++++++------
 include/test/log.h        |   1 +
 lib/Kconfig               |   5 +
 lib/Makefile              |   1 +
 lib/getopt.c              | 125 +++++++++++++
 test/lib/Makefile         |   1 +
 test/lib/getopt.c         | 123 +++++++++++++
 test/log/Makefile         |   1 +
 test/log/log_filter.c     | 111 ++++++++++++
 test/log/log_test.c       |  94 +++++++++-
 test/py/tests/test_log.py |  26 ++-
 19 files changed, 1253 insertions(+), 244 deletions(-)
 create mode 100644 doc/api/getopt.rst
 create mode 100644 include/getopt.h
 create mode 100644 lib/getopt.c
 create mode 100644 test/lib/getopt.c
 create mode 100644 test/log/log_filter.c

--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 01/22] log: Fix missing negation of ENOMEM

Sean Anderson
Errors returned should be negative.

Fixes: 45fac9fc18 ("log: Correct missing free() on error in log_add_filter()")

Signed-off-by: Sean Anderson <[hidden email]>
Reviewed-by: Heinrich Schuchardt <[hidden email]>
---

(no changes since v1)

 common/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/log.c b/common/log.c
index 9a5f100da3..3f6f4bdc2a 100644
--- a/common/log.c
+++ b/common/log.c
@@ -268,7 +268,7 @@ int log_add_filter(const char *drv_name, enum log_category_t cat_list[],
  if (file_list) {
  filt->file_list = strdup(file_list);
  if (!filt->file_list) {
- ret = ENOMEM;
+ ret = -ENOMEM;
  goto err;
  }
  }
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 02/22] log: Fix incorrect documentation of log_filter.cat_list

Sean Anderson
In reply to this post by Sean Anderson
Logging category lists are terminated by LOGC_END, not LOGC_NONE.

Fixes: e9c8d49d54 ("log: Add an implementation of logging")

Signed-off-by: Sean Anderson <[hidden email]>
Reviewed-by: Heinrich Schuchardt <[hidden email]>
---

Changes in v2:
- Also fix misdocumentation of for log_add_filter()

 include/log.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/log.h b/include/log.h
index 2859ce1f2e..1e29c94fd3 100644
--- a/include/log.h
+++ b/include/log.h
@@ -349,7 +349,7 @@ enum log_filter_flags {
  * new filter, and must be provided when removing a previously added
  * filter.
  * @flags: Flags for this filter (LOGFF_...)
- * @cat_list: List of categories to allow (terminated by LOGC_none). If empty
+ * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
  * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
  * can be provided
  * @max_level: Maximum log level to allow
@@ -423,7 +423,7 @@ int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
  *
  * @drv_name: Driver name to add the filter to (since each driver only has a
  * single device)
- * @cat_list: List of categories to allow (terminated by LOGC_none). If empty
+ * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
  * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
  * can be provided
  * @max_level: Maximum log level to allow
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 03/22] log: Add additional const qualifier to arrays

Sean Anderson
In reply to this post by Sean Anderson
Both these arrays and their members are const. Fixes checkpatch complaint.

Signed-off-by: Sean Anderson <[hidden email]>
---

Changes in v2:
- New

 common/log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/log.c b/common/log.c
index 3f6f4bdc2a..bda5139b69 100644
--- a/common/log.c
+++ b/common/log.c
@@ -13,7 +13,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static const char *log_cat_name[LOGC_COUNT - LOGC_NONE] = {
+static const char *const log_cat_name[LOGC_COUNT - LOGC_NONE] = {
  "none",
  "arch",
  "board",
@@ -23,7 +23,7 @@ static const char *log_cat_name[LOGC_COUNT - LOGC_NONE] = {
  "efi",
 };
 
-static const char *log_level_name[LOGL_COUNT] = {
+static const char *const log_level_name[LOGL_COUNT] = {
  "EMERG",
  "ALERT",
  "CRIT",
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 04/22] log: Add new category names to log_cat_name

Sean Anderson
In reply to this post by Sean Anderson
Without every category between LOGC_NONE and LOGC_COUNT present in
log_cat_name, log_get_cat_by_name will dereference NULL pointers if it
doesn't find a name early enough.

Fixes: c3aed5db59 ("sandbox: spi: Add more logging")
Fixes: a5c13b68e7 ("sandbox: log: Add a category for sandbox")
Fixes: 9f407d4ef0 ("Add core support for a bloblist to convey data from SPL")
Fixes: cce61fc428 ("dm: devres: Convert to use logging")
Fixes: 7ca2850cbc ("dm: core: Add basic ACPI support")

Signed-off-by: Sean Anderson <[hidden email]>
---

Changes in v2:
- Add compiletime assert on size of log_cat_name

 common/log.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/log.c b/common/log.c
index bda5139b69..11f488d71d 100644
--- a/common/log.c
+++ b/common/log.c
@@ -13,7 +13,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static const char *const log_cat_name[LOGC_COUNT - LOGC_NONE] = {
+static const char *const log_cat_name[] = {
  "none",
  "arch",
  "board",
@@ -21,6 +21,11 @@ static const char *const log_cat_name[LOGC_COUNT - LOGC_NONE] = {
  "driver-model",
  "device-tree",
  "efi",
+ "alloc",
+ "sandbox",
+ "bloblist",
+ "devres",
+ "acpi",
 };
 
 static const char *const log_level_name[LOGL_COUNT] = {
@@ -40,6 +45,9 @@ const char *log_get_cat_name(enum log_category_t cat)
 {
  const char *name;
 
+ compiletime_assert(ARRAY_SIZE(log_cat_name) == LOGC_COUNT - LOGC_NONE,
+   "missing logging category name");
+
  if (cat < 0 || cat >= LOGC_COUNT)
  return "<invalid>";
  if (cat >= LOGC_NONE)
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 05/22] log: Use CONFIG_IS_ENABLED() for LOG_TEST

Sean Anderson
In reply to this post by Sean Anderson
Checkpatch complains about using #ifdef for CONFIG variables.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 cmd/log.c           | 4 ++--
 test/log/log_test.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index 6afe6ead25..16a6ef7539 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -105,7 +105,7 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int argc,
 
 static struct cmd_tbl log_sub[] = {
  U_BOOT_CMD_MKENT(level, CONFIG_SYS_MAXARGS, 1, do_log_level, "", ""),
-#ifdef CONFIG_LOG_TEST
+#if CONFIG_IS_ENABLED(LOG_TEST)
  U_BOOT_CMD_MKENT(test, 2, 1, do_log_test, "", ""),
 #endif
  U_BOOT_CMD_MKENT(format, CONFIG_SYS_MAXARGS, 1, do_log_format, "", ""),
@@ -133,7 +133,7 @@ static int do_log(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
  "level - get/set log level\n"
-#ifdef CONFIG_LOG_TEST
+#if CONFIG_IS_ENABLED(LOG_TEST)
  "log test - run log tests\n"
 #endif
  "log format <fmt> - set log output format. <fmt> is a string where\n"
diff --git a/test/log/log_test.c b/test/log/log_test.c
index 4245372d65..787f680971 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -201,7 +201,7 @@ static int log_test(int testnum)
  return 0;
 }
 
-#ifdef CONFIG_LOG_TEST
+#if CONFIG_IS_ENABLED(LOG_TEST)
 int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
  int testnum = 0;
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 06/22] log: Expose some helper functions

Sean Anderson
In reply to this post by Sean Anderson
These functions are required by "cmd: log: Add commands to manipulate
filters" and "test: Add a test for log filter-*".

Signed-off-by: Sean Anderson <[hidden email]>
---

Changes in v2:
- Expose log_has_cat and log_has_file for filter tests

 common/log.c  | 23 +++--------------------
 include/log.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/common/log.c b/common/log.c
index 11f488d71d..5e9ce3184f 100644
--- a/common/log.c
+++ b/common/log.c
@@ -96,7 +96,7 @@ enum log_level_t log_get_level_by_name(const char *name)
  return LOGL_NONE;
 }
 
-static struct log_device *log_device_find_by_name(const char *drv_name)
+struct log_device *log_device_find_by_name(const char *drv_name)
 {
  struct log_device *ldev;
 
@@ -108,15 +108,7 @@ static struct log_device *log_device_find_by_name(const char *drv_name)
  return NULL;
 }
 
-/**
- * log_has_cat() - check if a log category exists within a list
- *
- * @cat_list: List of categories to check, at most LOGF_MAX_CATEGORIES entries
- * long, terminated by LC_END if fewer
- * @cat: Category to search for
- * @return true if @cat is in @cat_list, else false
- */
-static bool log_has_cat(enum log_category_t cat_list[], enum log_category_t cat)
+bool log_has_cat(enum log_category_t cat_list[], enum log_category_t cat)
 {
  int i;
 
@@ -128,16 +120,7 @@ static bool log_has_cat(enum log_category_t cat_list[], enum log_category_t cat)
  return false;
 }
 
-/**
- * log_has_file() - check if a file is with a list
- *
- * @file_list: List of files to check, separated by comma
- * @file: File to check for. This string is matched against the end of each
- * file in the list, i.e. ignoring any preceding path. The list is
- * intended to consist of relative pathnames, e.g. common/main.c,cmd/log.c
- * @return true if @file is in @file_list, else false
- */
-static bool log_has_file(const char *file_list, const char *file)
+bool log_has_file(const char *file_list, const char *file)
 {
  int file_len = strlen(file);
  const char *s, *p;
diff --git a/include/log.h b/include/log.h
index 1e29c94fd3..d84514a898 100644
--- a/include/log.h
+++ b/include/log.h
@@ -402,6 +402,37 @@ const char *log_get_level_name(enum log_level_t level);
  */
 enum log_level_t log_get_level_by_name(const char *name);
 
+/**
+ * log_device_find_by_name() - Look up a log device by its driver's name
+ *
+ * @drv_name: Name of the driver
+ * @return the log device, or NULL if not found
+ */
+struct log_device *log_device_find_by_name(const char *drv_name);
+
+/**
+ * log_has_cat() - check if a log category exists within a list
+ *
+ * @cat_list: List of categories to check, at most %LOGF_MAX_CATEGORIES entries
+ * long, terminated by %LC_END if fewer
+ * @cat: Category to search for
+ *
+ * Return: ``true`` if @cat is in @cat_list, else ``false``
+ */
+bool log_has_cat(enum log_category_t cat_list[], enum log_category_t cat);
+
+/**
+ * log_has_file() - check if a file is with a list
+ *
+ * @file_list: List of files to check, separated by comma
+ * @file: File to check for. This string is matched against the end of each
+ * file in the list, i.e. ignoring any preceding path. The list is
+ * intended to consist of relative pathnames, e.g. common/main.c,cmd/log.c
+ *
+ * Return: ``true`` if @file is in @file_list, else ``false``
+ */
+bool log_has_file(const char *file_list, const char *file);
+
 /* Log format flags (bit numbers) for gd->log_fmt. See log_fmt_chars */
 enum log_fmt {
  LOGF_CAT = 0,
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 07/22] log: Add function to create a filter with flags

Sean Anderson
In reply to this post by Sean Anderson
This function exposes a way to specify flags when creating a filter.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 common/log.c  |  6 ++++--
 include/log.h | 29 +++++++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/common/log.c b/common/log.c
index 5e9ce3184f..1066ead9e5 100644
--- a/common/log.c
+++ b/common/log.c
@@ -228,8 +228,9 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file,
  return 0;
 }
 
-int log_add_filter(const char *drv_name, enum log_category_t cat_list[],
-   enum log_level_t max_level, const char *file_list)
+int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
+ enum log_level_t max_level, const char *file_list,
+ int flags)
 {
  struct log_filter *filt;
  struct log_device *ldev;
@@ -243,6 +244,7 @@ int log_add_filter(const char *drv_name, enum log_category_t cat_list[],
  if (!filt)
  return -ENOMEM;
 
+ filt->flags = flags;
  if (cat_list) {
  filt->flags |= LOGFF_HAS_CAT;
  for (i = 0; ; i++) {
diff --git a/include/log.h b/include/log.h
index d84514a898..43935a1e34 100644
--- a/include/log.h
+++ b/include/log.h
@@ -449,6 +449,25 @@ enum log_fmt {
 /* Handle the 'log test' command */
 int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 
+/**
+ * log_add_filter_flags() - Add a new filter to a log device, specifying flags
+ *
+ * @drv_name: Driver name to add the filter to (since each driver only has a
+ * single device)
+ * @flags: Flags for this filter (LOGFF_...)
+ * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
+ * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
+ * can be provided
+ * @max_level: Maximum log level to allow
+ * @file_list: List of files to allow, separated by comma. If NULL then all
+ * files are permitted
+ * @return the sequence number of the new filter (>=0) if the filter was added,
+ * or a -ve value on error
+ */
+int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
+ enum log_level_t max_level, const char *file_list,
+ int flags);
+
 /**
  * log_add_filter() - Add a new filter to a log device
  *
@@ -463,8 +482,14 @@ int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
  * @return the sequence number of the new filter (>=0) if the filter was added,
  * or a -ve value on error
  */
-int log_add_filter(const char *drv_name, enum log_category_t cat_list[],
-   enum log_level_t max_level, const char *file_list);
+static inline int log_add_filter(const char *drv_name,
+ enum log_category_t cat_list[],
+ enum log_level_t max_level,
+ const char *file_list)
+{
+ return log_add_filter_flags(drv_name, cat_list, max_level, file_list,
+    0);
+}
 
 /**
  * log_remove_filter() - Remove a filter from a log device
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 08/22] log: Add filter flag to deny on match

Sean Anderson
In reply to this post by Sean Anderson
Without this flag, log filters can only explicitly accept messages.
Allowing denial makes it easier to filter certain subsystems. Unlike
allow-ing filters, deny-ing filters are added to the beginning of the
filter list. This should do the Right Thing most of the time, but it's
less-universal than allowing filters to be inserted anywhere. If this
becomes a problem, then perhaps log_filter_add* should take a filter number
to insert before/after.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 common/log.c  | 12 ++++++++++--
 include/log.h | 11 ++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/common/log.c b/common/log.c
index 1066ead9e5..910dc6b672 100644
--- a/common/log.c
+++ b/common/log.c
@@ -164,7 +164,11 @@ static bool log_passes_filters(struct log_device *ldev, struct log_rec *rec)
  if (filt->file_list &&
     !log_has_file(filt->file_list, rec->file))
  continue;
- return true;
+
+ if (filt->flags & LOGFF_DENY)
+ return false;
+ else
+ return true;
  }
 
  return false;
@@ -266,7 +270,11 @@ int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
  }
  }
  filt->filter_num = ldev->next_filter_num++;
- list_add_tail(&filt->sibling_node, &ldev->filter_head);
+ /* Add deny filters to the beginning of the list */
+ if (flags & LOGFF_DENY)
+ list_add(&filt->sibling_node, &ldev->filter_head);
+ else
+ list_add_tail(&filt->sibling_node, &ldev->filter_head);
 
  return filt->filter_num;
 
diff --git a/include/log.h b/include/log.h
index 43935a1e34..9f09d3a486 100644
--- a/include/log.h
+++ b/include/log.h
@@ -338,13 +338,22 @@ enum {
  LOGF_MAX_CATEGORIES = 5, /* maximum categories per filter */
 };
 
+/**
+ * enum log_filter_flags - Flags which modify a filter
+ */
 enum log_filter_flags {
- LOGFF_HAS_CAT = 1 << 0, /* Filter has a category list */
+ /** @LOGFF_HAS_CAT: Filter has a category list */
+ LOGFF_HAS_CAT = 1 << 0,
+ /** @LOGFF_DENY: Filter denies matching messages */
+ LOGFF_DENY = 1 << 1,
 };
 
 /**
  * struct log_filter - criterial to filter out log messages
  *
+ * If a message matches all criteria, then it is allowed. If LOGFF_DENY is set,
+ * then it is denied instead.
+ *
  * @filter_num: Sequence number of this filter.  This is returned when adding a
  * new filter, and must be provided when removing a previously added
  * filter.
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 09/22] test: Add tests for LOGFF_DENY

Sean Anderson
In reply to this post by Sean Anderson
This adds some tests for log filters which deny if they match.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 test/log/log_test.c       | 69 +++++++++++++++++++++++++++++++++++++++
 test/py/tests/test_log.py | 21 ++++++++++--
 2 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/test/log/log_test.c b/test/log/log_test.c
index 787f680971..6a71d3db7d 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -196,6 +196,75 @@ static int log_test(int testnum)
  log_io("level %d\n", LOGL_DEBUG_IO);
  break;
  }
+ case 11: {
+ /* Check denying based on category */
+ int filt1, filt2;
+ enum log_category_t cat_list[] = {
+ log_uc_cat(UCLASS_SPI), LOGC_END
+ };
+
+ ret = log_add_filter("console", cat_list, LOGL_MAX, NULL);
+ if (ret < 0)
+ return ret;
+ filt1 = ret;
+ ret = log_add_filter_flags("console", cat_list, LOGL_MAX, NULL,
+   LOGFF_DENY);
+ if (ret < 0)
+ return ret;
+ filt2 = ret;
+ log_run(UCLASS_SPI, "file");
+ ret = log_remove_filter("console", filt1);
+ if (ret < 0)
+ return ret;
+ ret = log_remove_filter("console", filt2);
+ if (ret < 0)
+ return ret;
+ break;
+ }
+ case 12: {
+ /* Check denying based on file */
+ int filt1, filt2;
+
+ ret = log_add_filter("console", NULL, LOGL_MAX, "file");
+ if (ret < 0)
+ return ret;
+ filt1 = ret;
+ ret = log_add_filter_flags("console", NULL, LOGL_MAX, "file",
+   LOGFF_DENY);
+ if (ret < 0)
+ return ret;
+ filt2 = ret;
+ log_run(UCLASS_SPI, "file");
+ ret = log_remove_filter("console", filt1);
+ if (ret < 0)
+ return ret;
+ ret = log_remove_filter("console", filt2);
+ if (ret < 0)
+ return ret;
+ break;
+ }
+ case 13: {
+ /* Check denying based on level */
+ int filt1, filt2;
+
+ ret = log_add_filter("console", NULL, LOGL_INFO, NULL);
+ if (ret < 0)
+ return ret;
+ filt1 = ret;
+ ret = log_add_filter_flags("console", NULL, LOGL_WARNING, NULL,
+   LOGFF_DENY);
+ if (ret < 0)
+ return ret;
+ filt2 = ret;
+ log_run(UCLASS_SPI, "file");
+ ret = log_remove_filter("console", filt1);
+ if (ret < 0)
+ return ret;
+ ret = log_remove_filter("console", filt2);
+ if (ret < 0)
+ return ret;
+ break;
+ }
  }
 
  return 0;
diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py
index ddc28f19ee..fabf3001cb 100644
--- a/test/py/tests/test_log.py
+++ b/test/py/tests/test_log.py
@@ -15,7 +15,8 @@ LOGL_FIRST, LOGL_WARNING, LOGL_INFO = (0, 4, 6)
 @pytest.mark.buildconfigspec('cmd_log')
 def test_log(u_boot_console):
     """Test that U-Boot logging works correctly."""
-    def check_log_entries(lines, mask, max_level=LOGL_INFO):
+    def check_log_entries(lines, mask, max_level=LOGL_INFO,
+          min_level=LOGL_FIRST):
         """Check that the expected log records appear in the output
 
         Args:
@@ -24,8 +25,9 @@ def test_log(u_boot_console):
                 bit 0: standard log line
                 bit 1: _log line
             max_level: maximum log level to expect in the output
+            min_level: minimum log level to expect in the output
         """
-        for i in range(max_level):
+        for i in range(min_level, max_level + 1):
             if mask & 1:
                 assert 'log_run() log %d' % i == next(lines)
             if mask & 3:
@@ -92,6 +94,18 @@ def test_log(u_boot_console):
         for i in range(7):
             assert 'log_test() level %d' % i == next(lines)
 
+    def test11():
+        lines = run_test(11)
+        assert next(lines, None) == None
+
+    def test12():
+        lines = run_test(12)
+        assert next(lines, None) == None
+
+    def test13():
+        lines = run_test(13)
+        check_log_entries(lines, 1, LOGL_INFO, LOGL_WARNING + 1)
+
     # TODO([hidden email]): Consider structuring this as separate tests
     cons = u_boot_console
     test0()
@@ -105,6 +119,9 @@ def test_log(u_boot_console):
     test8()
     test9()
     test10()
+    test11()
+    test12()
+    test13()
 
 @pytest.mark.buildconfigspec('cmd_log')
 def test_log_format(u_boot_console):
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 10/22] log: Add filter flag to match greater than a log level

Sean Anderson
In reply to this post by Sean Anderson
This is the compliment of the existing behavior to match only messages with
a log level less than a threshold. This is primarily useful in conjunction
with LOGFF_DENY.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 common/log.c  | 12 +++++++++---
 include/log.h | 10 ++++++----
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/common/log.c b/common/log.c
index 910dc6b672..7a9906a9b0 100644
--- a/common/log.c
+++ b/common/log.c
@@ -156,11 +156,17 @@ static bool log_passes_filters(struct log_device *ldev, struct log_rec *rec)
  }
 
  list_for_each_entry(filt, &ldev->filter_head, sibling_node) {
- if (rec->level > filt->max_level)
+ if (filt->flags & LOGFF_LEVEL_MIN) {
+ if (rec->level < filt->level)
+ continue;
+ } else if (rec->level > filt->level) {
  continue;
+ }
+
  if ((filt->flags & LOGFF_HAS_CAT) &&
     !log_has_cat(filt->cat_list, rec->cat))
  continue;
+
  if (filt->file_list &&
     !log_has_file(filt->file_list, rec->file))
  continue;
@@ -233,7 +239,7 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file,
 }
 
 int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
- enum log_level_t max_level, const char *file_list,
+ enum log_level_t level, const char *file_list,
  int flags)
 {
  struct log_filter *filt;
@@ -261,7 +267,7 @@ int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
  break;
  }
  }
- filt->max_level = max_level;
+ filt->level = level;
  if (file_list) {
  filt->file_list = strdup(file_list);
  if (!filt->file_list) {
diff --git a/include/log.h b/include/log.h
index 9f09d3a486..9dd4484095 100644
--- a/include/log.h
+++ b/include/log.h
@@ -346,6 +346,8 @@ enum log_filter_flags {
  LOGFF_HAS_CAT = 1 << 0,
  /** @LOGFF_DENY: Filter denies matching messages */
  LOGFF_DENY = 1 << 1,
+ /** @LOGFF_LEVEL_MIN: Filter's level is a minimum, not a maximum */
+ LOGFF_LEVEL_MIN = 1 << 2,
 };
 
 /**
@@ -361,7 +363,7 @@ enum log_filter_flags {
  * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
  * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
  * can be provided
- * @max_level: Maximum log level to allow
+ * @level: Maximum (or minimum, if LOGFF_MIN_LEVEL) log level to allow
  * @file_list: List of files to allow, separated by comma. If NULL then all
  * files are permitted
  * @sibling_node: Next filter in the list of filters for this log device
@@ -370,7 +372,7 @@ struct log_filter {
  int filter_num;
  int flags;
  enum log_category_t cat_list[LOGF_MAX_CATEGORIES];
- enum log_level_t max_level;
+ enum log_level_t level;
  const char *file_list;
  struct list_head sibling_node;
 };
@@ -467,14 +469,14 @@ int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
  * @cat_list: List of categories to allow (terminated by %LOGC_END). If empty
  * then all categories are permitted. Up to LOGF_MAX_CATEGORIES entries
  * can be provided
- * @max_level: Maximum log level to allow
+ * @level: Maximum (or minimum, if LOGFF_LEVEL_MIN) log level to allow
  * @file_list: List of files to allow, separated by comma. If NULL then all
  * files are permitted
  * @return the sequence number of the new filter (>=0) if the filter was added,
  * or a -ve value on error
  */
 int log_add_filter_flags(const char *drv_name, enum log_category_t cat_list[],
- enum log_level_t max_level, const char *file_list,
+ enum log_level_t level, const char *file_list,
  int flags);
 
 /**
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 11/22] test: Add test for LOGFF_MIN

Sean Anderson
In reply to this post by Sean Anderson
This tests log filters matching on a minimum level.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 test/log/log_test.c       | 23 +++++++++++++++++++++++
 test/py/tests/test_log.py |  5 +++++
 2 files changed, 28 insertions(+)

diff --git a/test/log/log_test.c b/test/log/log_test.c
index 6a71d3db7d..da433e332b 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -265,6 +265,29 @@ static int log_test(int testnum)
  return ret;
  break;
  }
+ case 14: {
+ /* Check matching based on minimum level */
+ int filt1, filt2;
+
+ ret = log_add_filter_flags("console", NULL, LOGL_WARNING, NULL,
+   LOGFF_LEVEL_MIN);
+ if (ret < 0)
+ return ret;
+ filt1 = ret;
+ ret = log_add_filter_flags("console", NULL, LOGL_INFO, NULL,
+   LOGFF_DENY | LOGFF_LEVEL_MIN);
+ if (ret < 0)
+ return ret;
+ filt2 = ret;
+ log_run(UCLASS_SPI, "file");
+ ret = log_remove_filter("console", filt1);
+ if (ret < 0)
+ return ret;
+ ret = log_remove_filter("console", filt2);
+ if (ret < 0)
+ return ret;
+ break;
+ }
  }
 
  return 0;
diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py
index fabf3001cb..f191c84c02 100644
--- a/test/py/tests/test_log.py
+++ b/test/py/tests/test_log.py
@@ -106,6 +106,10 @@ def test_log(u_boot_console):
         lines = run_test(13)
         check_log_entries(lines, 1, LOGL_INFO, LOGL_WARNING + 1)
 
+    def test14():
+        lines = run_test(14)
+        check_log_entries(lines, 1, LOGL_INFO - 1, LOGL_WARNING)
+
     # TODO([hidden email]): Consider structuring this as separate tests
     cons = u_boot_console
     test0()
@@ -122,6 +126,7 @@ def test_log(u_boot_console):
     test11()
     test12()
     test13()
+    test14()
 
 @pytest.mark.buildconfigspec('cmd_log')
 def test_log_format(u_boot_console):
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 12/22] cmd: log: Use sub-commands for log

Sean Anderson
In reply to this post by Sean Anderson
This reduces duplicate code, and makes adding new sub-commands easier.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 cmd/log.c | 37 +++++++------------------------------
 1 file changed, 7 insertions(+), 30 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index 16a6ef7539..e55ace9e14 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -103,33 +103,6 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int argc,
  return 0;
 }
 
-static struct cmd_tbl log_sub[] = {
- U_BOOT_CMD_MKENT(level, CONFIG_SYS_MAXARGS, 1, do_log_level, "", ""),
-#if CONFIG_IS_ENABLED(LOG_TEST)
- U_BOOT_CMD_MKENT(test, 2, 1, do_log_test, "", ""),
-#endif
- U_BOOT_CMD_MKENT(format, CONFIG_SYS_MAXARGS, 1, do_log_format, "", ""),
- U_BOOT_CMD_MKENT(rec, CONFIG_SYS_MAXARGS, 1, do_log_rec, "", ""),
-};
-
-static int do_log(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
-{
- struct cmd_tbl *cp;
-
- if (argc < 2)
- return CMD_RET_USAGE;
-
- /* drop initial "log" arg */
- argc--;
- argv++;
-
- cp = find_cmd_tbl(argv[0], log_sub, ARRAY_SIZE(log_sub));
- if (cp)
- return cp->cmd(cmdtp, flag, argc, argv);
-
- return CMD_RET_USAGE;
-}
-
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
  "level - get/set log level\n"
@@ -145,7 +118,11 @@ static char log_help_text[] =
  ;
 #endif
 
-U_BOOT_CMD(
- log, CONFIG_SYS_MAXARGS, 1, do_log,
- "log system", log_help_text
+U_BOOT_CMD_WITH_SUBCMDS(log, "log system", log_help_text,
+ U_BOOT_SUBCMD_MKENT(level, 2, 1, do_log_level),
+#if CONFIG_IS_ENABLED(LOG_TEST)
+ U_BOOT_SUBCMD_MKENT(test, 2, 1, do_log_test),
+#endif
+ U_BOOT_SUBCMD_MKENT(format, 2, 1, do_log_format),
+ U_BOOT_SUBCMD_MKENT(rec, 7, 1, do_log_rec),
 );
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 13/22] cmd: log: Split off log level parsing

Sean Anderson
In reply to this post by Sean Anderson
Move parsing of log level into its own function so it can be re-used. This
also adds support for using log level names instead of just the integer
equivalent.

Signed-off-by: Sean Anderson <[hidden email]>
---

Changes in v2:
- Print an error message if the log level is invalid.

 cmd/log.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index e55ace9e14..5387aab551 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -11,23 +11,40 @@
 
 static char log_fmt_chars[LOGF_COUNT] = "clFLfm";
 
+static enum log_level_t parse_log_level(char *const arg)
+{
+ enum log_level_t ret;
+ ulong level;
+
+ if (!strict_strtoul(arg, 10, &level)) {
+ if (level > _LOG_MAX_LEVEL) {
+ printf("Only log levels <= %d are supported\n",
+       _LOG_MAX_LEVEL);
+ return LOGL_NONE;
+ }
+ return level;
+ }
+
+ ret = log_get_level_by_name(arg);
+ if (ret == LOGL_NONE)
+ printf("Unknown log level \"%s\"\n", arg);
+ return ret;
+}
+
 static int do_log_level(struct cmd_tbl *cmdtp, int flag, int argc,
  char *const argv[])
 {
  if (argc > 1) {
- long log_level = simple_strtol(argv[1], NULL, 10);
+ enum log_level_t log_level = parse_log_level(argv[1]);
 
- if (log_level < 0 || log_level > _LOG_MAX_LEVEL) {
- printf("Only log levels <= %d are supported\n",
-       _LOG_MAX_LEVEL);
+ if (log_level == LOGL_NONE)
  return CMD_RET_FAILURE;
- }
  gd->default_log_level = log_level;
  } else {
  printf("Default log level: %d\n", gd->default_log_level);
  }
 
- return 0;
+ return CMD_RET_SUCCESS;
 }
 
 static int do_log_format(struct cmd_tbl *cmdtp, int flag, int argc,
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 14/22] cmd: log: Move log test to end of help string

Sean Anderson
In reply to this post by Sean Anderson
This is probably what a typical user is least interested in. Move it to the
end of the description.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 cmd/log.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index 5387aab551..9076e220f0 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -123,23 +123,23 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int argc,
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
  "level - get/set log level\n"
-#if CONFIG_IS_ENABLED(LOG_TEST)
- "log test - run log tests\n"
-#endif
  "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"
  "\tor 'default', or 'all' for all\n"
  "log rec <category> <level> <file> <line> <func> <message> - "
  "output a log record"
+#if CONFIG_IS_ENABLED(LOG_TEST)
+ "\nlog test - run log tests"
+#endif
  ;
 #endif
 
 U_BOOT_CMD_WITH_SUBCMDS(log, "log system", log_help_text,
  U_BOOT_SUBCMD_MKENT(level, 2, 1, do_log_level),
+ U_BOOT_SUBCMD_MKENT(format, 2, 1, do_log_format),
+ U_BOOT_SUBCMD_MKENT(rec, 7, 1, do_log_rec),
 #if CONFIG_IS_ENABLED(LOG_TEST)
  U_BOOT_SUBCMD_MKENT(test, 2, 1, do_log_test),
 #endif
- U_BOOT_SUBCMD_MKENT(format, 2, 1, do_log_format),
- U_BOOT_SUBCMD_MKENT(rec, 7, 1, do_log_rec),
 );
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 15/22] cmd: log: Add commands to list categories and drivers

Sean Anderson
In reply to this post by Sean Anderson
This allows users to query which categories and drivers are available on
their system. This allows them to construct filter-add commands without
(e.g.) adjusting the log format to show categories and drivers.

Signed-off-by: Sean Anderson <[hidden email]>
---

Changes in v2:
- New

 cmd/log.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/cmd/log.c b/cmd/log.c
index 9076e220f0..ea458f36f4 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -47,6 +47,38 @@ static int do_log_level(struct cmd_tbl *cmdtp, int flag, int argc,
  return CMD_RET_SUCCESS;
 }
 
+static int do_log_categories(struct cmd_tbl *cmdtp, int flag, int argc,
+     char *const argv[])
+{
+ enum log_category_t cat;
+ const char *name;
+
+ for (cat = LOGC_FIRST; cat < LOGC_COUNT; cat++) {
+ name = log_get_cat_name(cat);
+ /*
+ * Invalid category names (e.g. <invalid> or <missing>) begin
+ * with '<'. TODO: perhaps change the interface of
+ * log_get_cat_name to return an error code?
+ */
+ if (name[0] == '<')
+ continue;
+ printf("%s\n", name);
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
+static int do_log_drivers(struct cmd_tbl *cmdtp, int flag, int argc,
+  char *const argv[])
+{
+ struct log_device *ldev;
+
+ list_for_each_entry(ldev, &gd->log_head, sibling_node)
+ printf("%s\n", ldev->drv->name);
+
+ return CMD_RET_SUCCESS;
+}
+
 static int do_log_format(struct cmd_tbl *cmdtp, int flag, int argc,
  char *const argv[])
 {
@@ -123,6 +155,8 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int argc,
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
  "level - get/set log level\n"
+ "categories - list log categories\n"
+ "drivers - list log drivers\n"
  "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"
@@ -137,6 +171,8 @@ static char log_help_text[] =
 
 U_BOOT_CMD_WITH_SUBCMDS(log, "log system", log_help_text,
  U_BOOT_SUBCMD_MKENT(level, 2, 1, do_log_level),
+ U_BOOT_SUBCMD_MKENT(categories, 1, 1, do_log_categories),
+ U_BOOT_SUBCMD_MKENT(drivers, 1, 1, do_log_drivers),
  U_BOOT_SUBCMD_MKENT(format, 2, 1, do_log_format),
  U_BOOT_SUBCMD_MKENT(rec, 7, 1, do_log_rec),
 #if CONFIG_IS_ENABLED(LOG_TEST)
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 16/22] cmd: log: Make "log level" print all log levels

Sean Anderson
In reply to this post by Sean Anderson
This makes the log level command print all valid log levels. The default
log level is annotated. This provides an easy way to see which log levels
are compiled-in.

Signed-off-by: Sean Anderson <[hidden email]>
---

Changes in v2:
- New

 cmd/log.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/cmd/log.c b/cmd/log.c
index ea458f36f4..f3a265798c 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -34,14 +34,20 @@ static enum log_level_t parse_log_level(char *const arg)
 static int do_log_level(struct cmd_tbl *cmdtp, int flag, int argc,
  char *const argv[])
 {
+ enum log_level_t log_level;
+
  if (argc > 1) {
- enum log_level_t log_level = parse_log_level(argv[1]);
+ log_level = parse_log_level(argv[1]);
 
  if (log_level == LOGL_NONE)
  return CMD_RET_FAILURE;
  gd->default_log_level = log_level;
  } else {
- printf("Default log level: %d\n", gd->default_log_level);
+ for (log_level = LOGL_FIRST; log_level <= _LOG_MAX_LEVEL;
+     log_level++)
+ printf("%s%s\n", log_get_level_name(log_level),
+       log_level == gd->default_log_level ?
+       " (default)" : "");
  }
 
  return CMD_RET_SUCCESS;
@@ -154,7 +160,7 @@ static int do_log_rec(struct cmd_tbl *cmdtp, int flag, int argc,
 
 #ifdef CONFIG_SYS_LONGHELP
 static char log_help_text[] =
- "level - get/set log level\n"
+ "level [<level>] - get/set log level\n"
  "categories - list log categories\n"
  "drivers - list log drivers\n"
  "log format <fmt> - set log output format. <fmt> is a string where\n"
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 17/22] lib: Add getopt

Sean Anderson
In reply to this post by Sean Anderson
Some commands can get very unweildy if they have too many positional
arguments. Adding options makes them easier to read, remember, and
understand.

This implementation of getopt has been taken from barebox, which has had
option support for quite a while. I have made a few modifications to their
version, such as the removal of opterr in favor of a separate getopt_silent
function. In addition, I have moved all global variables into struct
getopt_context.

The getopt from barebox also re-orders the arguments passed to it so that
non-options are placed last. This allows users to specify options anywhere.
For example, `ls -l foo/ -R` would be re-ordered to `ls -l -R foo/` as
getopt parsed the options. However, this feature conflicts with the const
argv in cmd_tbl->cmd. This was originally added in 54841ab50c ("Make sure
that argv[] argument pointers are not modified."). The reason stated in
that commit is that hush requires argv to stay unmodified. Has this
situation changed? Barebox also uses hush, and does not have this problem.
Perhaps we could use their fix?

I have assigned maintenance of getopt to Simon Glass, as it is currently
only used by the log command. I would also be fine maintaining it.

Signed-off-by: Sean Anderson <[hidden email]>
---

Changes in v2:
- Expand documentation of getopt() to include examples
- Remove opt prefix from getopt_state members

 MAINTAINERS        |   1 +
 doc/api/getopt.rst |   8 +++
 doc/api/index.rst  |   1 +
 include/getopt.h   | 130 +++++++++++++++++++++++++++++++++++++++++++++
 lib/Kconfig        |   5 ++
 lib/Makefile       |   1 +
 lib/getopt.c       | 125 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 271 insertions(+)
 create mode 100644 doc/api/getopt.rst
 create mode 100644 include/getopt.h
 create mode 100644 lib/getopt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index fb9ba37984..0f8e1f6d4c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -747,6 +747,7 @@ T: git https://gitlab.denx.de/u-boot/u-boot.git
 F: common/log*
 F: cmd/log.c
 F: doc/develop/logging.rst
+F: lib/getopt.c
 F: test/log/
 F: test/py/tests/test_log.py
 
diff --git a/doc/api/getopt.rst b/doc/api/getopt.rst
new file mode 100644
index 0000000000..773f79aeb6
--- /dev/null
+++ b/doc/api/getopt.rst
@@ -0,0 +1,8 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright (C) 2020 Sean Anderson <[hidden email]>
+
+Option Parsing
+==============
+
+.. kernel-doc:: include/getopt.h
+   :internal:
diff --git a/doc/api/index.rst b/doc/api/index.rst
index 1c261bcb73..d90e70e16a 100644
--- a/doc/api/index.rst
+++ b/doc/api/index.rst
@@ -8,6 +8,7 @@ U-Boot API documentation
 
    dfu
    efi
+   getopt
    linker_lists
    pinctrl
    rng
diff --git a/include/getopt.h b/include/getopt.h
new file mode 100644
index 0000000000..6f5811e64b
--- /dev/null
+++ b/include/getopt.h
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * getopt.h - a simple getopt(3) implementation.
+ *
+ * Copyright (C) 2020 Sean Anderson <[hidden email]>
+ * Copyright (c) 2007 Sascha Hauer <[hidden email]>, Pengutronix
+ */
+
+#ifndef __GETOPT_H
+#define __GETOPT_H
+
+/**
+ * struct getopt_state - Saved state across getopt() calls
+ */
+struct getopt_state {
+ /**
+ * @index: Index of the next unparsed argument of @argv. If getopt() has
+ * parsed all of @argv, then @index will equal @argc.
+ */
+ int index;
+ /* private: */
+ /** @arg_index: Index within the current argument */
+ int arg_index;
+ union {
+ /* public: */
+ /**
+ * @opt: Option being parsed when an error occurs. @opt is only
+ * valid when getopt() returns ``?`` or ``:``.
+ */
+ int opt;
+ /**
+ * @arg: The argument to an option, NULL if there is none. @arg
+ * is only valid when getopt() returns an option character.
+ */
+ char *arg;
+ /* private: */
+ };
+};
+
+/**
+ * getopt_init_state() - Initialize a &struct getopt_state
+ * @gs: The state to initialize
+ *
+ * This must be called before using @gs with getopt().
+ */
+void getopt_init_state(struct getopt_state *gs);
+
+int __getopt(struct getopt_state *gs, int argc, char *const argv[],
+     const char *optstring, bool silent);
+
+/**
+ * getopt() - Parse short command-line options
+ * @gs: Internal state and out-of-band return arguments. This must be
+ *      initialized with getopt_init_context() beforehand.
+ * @argc: Number of arguments, not including the %NULL terminator
+ * @argv: Argument list, terminated by %NULL
+ * @optstring: Option specification, as described below
+ *
+ * getopt() parses short options. Short options are single characters. They may
+ * be followed by a required argument or an optional argument. Arguments to
+ * options may occur in the same argument as an option (like ``-larg``), or
+ * in the following argument (like ``-l arg``). An argument containing
+ * options begins with a ``-``. If an option expects no arguments, then it may
+ * be immediately followed by another option (like ``ls -alR``).
+ *
+ * @optstring is a list of accepted options. If an option is followed by ``:``
+ * in @optstring, then it expects a mandatory argument. If an option is followed
+ * by ``::`` in @optstring, it expects an optional argument. @gs.arg points
+ * to the argument, if one is parsed.
+ *
+ * getopt() stops parsing options when it encounters the first non-option
+ * argument, when it encounters the argument ``--``, or when it runs out of
+ * arguments. For example, in ``ls -l foo -R``, option parsing will stop when
+ * getopt() encounters ``foo``, if ``l`` does not expect an argument. However,
+ * the whole list of arguments would be parsed if ``l`` expects an argument.
+ *
+ * An example invocation of getopt() might look like::
+ *
+ *     char *argv[] = { "program", "-cbx", "-a", "foo", "bar", 0 };
+ *     int opt, argc = ARRAY_SIZE(argv) - 1;
+ *     struct getopt_state gs;
+ *
+ *     getopt_init_state(&gs);
+ *     while ((opt = getopt(&gs, argc, argv, "a::b:c")) != -1)
+ *         printf("opt = %c, index = %d, arg = \"%s\"\n", opt, gs.index, gs.arg);
+ *     printf("%d argument(s) left\n", argc - gs.index);
+ *
+ * and would produce an output of::
+ *
+ *     opt = c, index = 1, arg = "<NULL>"
+ *     opt = b, index = 2, arg = "x"
+ *     opt = a, index = 4, arg = "foo"
+ *     1 argument(s) left
+ *
+ * For further information, refer to the getopt(3) man page.
+ *
+ * Return:
+ * * An option character if an option is found. @gs.arg is set to the
+ *   argument if there is one, otherwise it is set to ``NULL``.
+ * * ``-1`` if there are no more options, if a non-option argument is
+ *   encountered, or if an ``--`` argument is encountered.
+ * * ``'?'`` if we encounter an option not in @optstring. @gs.opt is set to
+ *   the unknown option.
+ * * ``':'`` if an argument is required, but no argument follows the
+ *   option. @gs.opt is set to the option missing its argument.
+ *
+ * @gs.index is always set to the index of the next unparsed argument in @argv.
+ */
+static inline int getopt(struct getopt_state *gs, int argc,
+ char *const argv[], const char *optstring)
+{
+ return __getopt(gs, argc, argv, optstring, false);
+}
+
+/**
+ * getopt_silent() - Parse short command-line options silently
+ * @gs: State
+ * @argc: Argument count
+ * @argv: Argument list
+ * @optstring: Option specification
+ *
+ * Same as getopt(), except no error messages are printed.
+ */
+static inline int getopt_silent(struct getopt_state *gs, int argc,
+ char *const argv[], const char *optstring)
+{
+ return __getopt(gs, argc, argv, optstring, true);
+}
+
+#endif /* __GETOPT_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 8efb154f73..a3346eee04 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -542,6 +542,11 @@ config HEXDUMP
  help
   This enables functions for printing dumps of binary data.
 
+config GETOPT
+ bool "Enable getopt"
+ help
+  This enables functions for parsing command-line options.
+
 config OF_LIBFDT
  bool "Enable the FDT library"
  default y if OF_CONTROL
diff --git a/lib/Makefile b/lib/Makefile
index 0cd7bea282..7c7fb9aae7 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -106,6 +106,7 @@ obj-y += string.o
 obj-y += tables_csum.o
 obj-y += time.o
 obj-y += hexdump.o
+obj-$(CONFIG_GETOPT) += getopt.o
 obj-$(CONFIG_TRACE) += trace.o
 obj-$(CONFIG_LIB_UUID) += uuid.o
 obj-$(CONFIG_LIB_RAND) += rand.o
diff --git a/lib/getopt.c b/lib/getopt.c
new file mode 100644
index 0000000000..8b4515dc19
--- /dev/null
+++ b/lib/getopt.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * getopt.c - a simple getopt(3) implementation. See getopt.h for explanation.
+ *
+ * Copyright (C) 2020 Sean Anderson <[hidden email]>
+ * Copyright (c) 2007 Sascha Hauer <[hidden email]>, Pengutronix
+ */
+
+#define LOG_CATEGORY LOGC_CORE
+
+#include <common.h>
+#include <getopt.h>
+#include <log.h>
+
+void getopt_init_state(struct getopt_state *gs)
+{
+ gs->index = 1;
+ gs->arg_index = 1;
+}
+
+int __getopt(struct getopt_state *gs, int argc, char *const argv[],
+     const char *optstring, bool silent)
+{
+ char curopt;   /* current option character */
+ const char *curoptp; /* pointer to the current option in optstring */
+
+ while (1) {
+ log_debug("arg_index: %d index: %d\n", gs->arg_index,
+  gs->index);
+
+ /* `--` indicates the end of options */
+ if (gs->arg_index == 1 && argv[gs->index] &&
+    !strcmp(argv[gs->index], "--")) {
+ gs->index++;
+ return -1;
+ }
+
+ /* Out of arguments */
+ if (gs->index >= argc)
+ return -1;
+
+ /* Can't parse non-options */
+ if (*argv[gs->index] != '-')
+ return -1;
+
+ /* We have found an option */
+ curopt = argv[gs->index][gs->arg_index];
+ if (curopt)
+ break;
+ /*
+ * no more options in current argv[] element; try the next one
+ */
+ gs->index++;
+ gs->arg_index = 1;
+ }
+
+ /* look up current option in optstring */
+ curoptp = strchr(optstring, curopt);
+
+ if (!curoptp) {
+ if (!silent)
+ printf("%s: invalid option -- %c\n", argv[0], curopt);
+ gs->opt = curopt;
+ gs->arg_index++;
+ return '?';
+ }
+
+ if (*(curoptp + 1) != ':') {
+ /* option with no argument. Just return it */
+ gs->arg = NULL;
+ gs->arg_index++;
+ return curopt;
+ }
+
+ if (*(curoptp + 1) && *(curoptp + 2) == ':') {
+ /* optional argument */
+ if (argv[gs->index][gs->arg_index + 1]) {
+ /* optional argument with directly following arg */
+ gs->arg = argv[gs->index++] + gs->arg_index + 1;
+ gs->arg_index = 1;
+ return curopt;
+ }
+ if (gs->index + 1 == argc) {
+ /* We are at the last argv[] element */
+ gs->arg = NULL;
+ gs->index++;
+ return curopt;
+ }
+ if (*argv[gs->index + 1] != '-') {
+ /*
+ * optional argument with arg in next argv[] element
+ */
+ gs->index++;
+ gs->arg = argv[gs->index++];
+ gs->arg_index = 1;
+ return curopt;
+ }
+
+ /* no optional argument found */
+ gs->arg = NULL;
+ gs->arg_index = 1;
+ gs->index++;
+ return curopt;
+ }
+
+ if (argv[gs->index][gs->arg_index + 1]) {
+ /* required argument with directly following arg */
+ gs->arg = argv[gs->index++] + gs->arg_index + 1;
+ gs->arg_index = 1;
+ return curopt;
+ }
+
+ gs->index++;
+ gs->arg_index = 1;
+
+ if (gs->index >= argc || argv[gs->index][0] == '-') {
+ if (!silent)
+ printf("option requires an argument -- %c\n", curopt);
+ gs->opt = curopt;
+ return ':';
+ }
+
+ gs->arg = argv[gs->index++];
+ return curopt;
+}
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 18/22] test: Add a test for getopt

Sean Anderson
In reply to this post by Sean Anderson
A few of these tests were inspired by those in glibc. The syntax for
invoking test_getopt is a bit funky, but it's necessary so that the CPP can
parse the arguments correctly.

Signed-off-by: Sean Anderson <[hidden email]>
---

(no changes since v1)

 test/lib/Makefile |   1 +
 test/lib/getopt.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)
 create mode 100644 test/lib/getopt.c

diff --git a/test/lib/Makefile b/test/lib/Makefile
index 22236f8587..3140c2dad7 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_ERRNO_STR) += test_errno_str.o
 obj-$(CONFIG_UT_LIB_ASN1) += asn1.o
 obj-$(CONFIG_UT_LIB_RSA) += rsa.o
 obj-$(CONFIG_AES) += test_aes.o
+obj-$(CONFIG_GETOPT) += getopt.o
diff --git a/test/lib/getopt.c b/test/lib/getopt.c
new file mode 100644
index 0000000000..3c68b93c8a
--- /dev/null
+++ b/test/lib/getopt.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Sean Anderson <[hidden email]>
+ *
+ * Portions of these tests were inspired by glibc's posix/bug-getopt1.c and
+ * posix/tst-getopt-cancel.c
+ */
+
+#include <common.h>
+#include <getopt.h>
+#include <test/lib.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+static int do_test_getopt(struct unit_test_state *uts, int line,
+  struct getopt_state *gs, const char *optstring,
+  int args, char *argv[], int expected_count,
+  int expected[])
+{
+ int opt;
+
+ getopt_init_state(gs);
+ for (int i = 0; i < expected_count; i++) {
+ opt = getopt_silent(gs, args, argv, optstring);
+ if (expected[i] != opt) {
+ /*
+ * Fudge the line number so we can tell which test
+ * failed
+ */
+ ut_failf(uts, __FILE__, line, __func__,
+ "expected[i] == getopt()",
+ "Expected '%c' (%d) with i=%d, got '%c' (%d)",
+ expected[i], expected[i], i, opt, opt);
+ return CMD_RET_FAILURE;
+ }
+ }
+
+ opt = getopt_silent(gs, args, argv, optstring);
+ if (opt != -1) {
+ ut_failf(uts, __FILE__, line, __func__,
+ "getopt() != -1",
+ "Expected -1, got '%c' (%d)", opt, opt);
+ return CMD_RET_FAILURE;
+ }
+
+ return 0;
+}
+
+#define test_getopt(optstring, argv, expected) do { \
+ int ret = do_test_getopt(uts, __LINE__, &gs, optstring, \
+ ARRAY_SIZE(argv) - 1, argv, \
+ ARRAY_SIZE(expected), expected); \
+ if (ret) \
+ return ret; \
+} while (0)
+
+static int lib_test_getopt(struct unit_test_state *uts)
+{
+ struct getopt_state gs;
+
+ /* Happy path */
+ test_getopt("ab:c",
+    ((char *[]){ "program", "-cb", "x", "-a", "foo", 0 }),
+    ((int []){ 'c', 'b', 'a' }));
+ ut_asserteq(4, gs.index);
+
+ /* Make sure we pick up the optional argument */
+ test_getopt("a::b:c",
+    ((char *[]){ "program", "-cbx", "-a", "foo", 0 }),
+    ((int []){ 'c', 'b', 'a' }));
+ ut_asserteq(4, gs.index);
+
+ /* Test required arguments */
+ test_getopt("a:b", ((char *[]){ "program", "-a", 0 }),
+    ((int []){ ':' }));
+ ut_asserteq('a', gs.opt);
+ test_getopt("a:b", ((char *[]){ "program", "-b", "-a", 0 }),
+    ((int []){ 'b', ':' }));
+ ut_asserteq('a', gs.opt);
+
+ /* Test invalid arguments */
+ test_getopt("ab:c", ((char *[]){ "program", "-d", 0 }),
+    ((int []){ '?' }));
+ ut_asserteq('d', gs.opt);
+
+ /* Test arg */
+ test_getopt("a::b:c",
+    ((char *[]){ "program", "-a", 0 }),
+    ((int []){ 'a' }));
+ ut_asserteq(2, gs.index);
+ ut_assertnull(gs.arg);
+
+ test_getopt("a::b:c",
+    ((char *[]){ "program", "-afoo", 0 }),
+    ((int []){ 'a' }));
+ ut_asserteq(2, gs.index);
+ ut_assertnonnull(gs.arg);
+ ut_asserteq_str("foo", gs.arg);
+
+ test_getopt("a::b:c",
+    ((char *[]){ "program", "-a", "foo", 0 }),
+    ((int []){ 'a' }));
+ ut_asserteq(3, gs.index);
+ ut_assertnonnull(gs.arg);
+ ut_asserteq_str("foo", gs.arg);
+
+ test_getopt("a::b:c",
+    ((char *[]){ "program", "-bfoo", 0 }),
+    ((int []){ 'b' }));
+ ut_asserteq(2, gs.index);
+ ut_assertnonnull(gs.arg);
+ ut_asserteq_str("foo", gs.arg);
+
+ test_getopt("a::b:c",
+    ((char *[]){ "program", "-b", "foo", 0 }),
+    ((int []){ 'b' }));
+ ut_asserteq(3, gs.index);
+ ut_assertnonnull(gs.arg);
+ ut_asserteq_str("foo", gs.arg);
+
+ return 0;
+}
+LIB_TEST(lib_test_getopt, 0);
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 19/22] cmd: log: Add commands to manipulate filters

Sean Anderson
In reply to this post by Sean Anderson
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 <[hidden email]>
---

Changes in v2:
- Add option to remove all filters to filter-remove
- Clarify filter-* help text

 cmd/Kconfig |   1 +
 cmd/log.c   | 240 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 241 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 4cb171790b..e7e5d88bdb 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2147,6 +2147,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 f3a265798c..3c27b35025 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -7,7 +7,9 @@
 #include <common.h>
 #include <command.h>
 #include <dm.h>
+#include <getopt.h>
 #include <log.h>
+#include <malloc.h>
 
 static char log_fmt_chars[LOGF_COUNT] = "clFLfm";
 
@@ -85,6 +87,220 @@ static int do_log_drivers(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.arg;
+ break;
+ default:
+ return CMD_RET_USAGE;
+ }
+ }
+
+ if (gs.index != 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.arg);
+ if (cat == LOGC_NONE) {
+ printf("Unknown category \"%s\"\n", gs.arg);
+ return CMD_RET_FAILURE;
+ }
+
+ cat_list[cat_count++] = cat;
+ break;
+ }
+ case 'd':
+ drv_name = gs.arg;
+ break;
+ case 'D':
+ do_type();
+ flags |= LOGFF_DENY;
+ break;
+ case 'f':
+ file_list = gs.arg;
+ 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.arg); \
+ 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.index != 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[])
+{
+ bool all = false;
+ 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, "ad:")) > 0) {
+ switch (opt) {
+ case 'a':
+ all = true;
+ break;
+ case 'd':
+ drv_name = gs.arg;
+ break;
+ default:
+ return CMD_RET_USAGE;
+ }
+ }
+
+ if (all) {
+ struct log_filter *filt, *tmp_filt;
+ struct log_device *ldev;
+
+ if (gs.index != 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;
+ }
+
+ list_for_each_entry_safe(filt, tmp_filt, &ldev->filter_head,
+ sibling_node) {
+ list_del(&filt->sibling_node);
+ free(filt);
+ }
+ } else {
+ if (gs.index + 1 != argc)
+ return CMD_RET_USAGE;
+
+ if (strict_strtoul(argv[gs.index], 10, &filter_num)) {
+ printf("Invalid filter number \"%s\"\n", argv[gs.index]);
+ 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[])
 {
@@ -163,6 +379,26 @@ static char log_help_text[] =
  "level [<level>] - get/set log level\n"
  "categories - list log categories\n"
  "drivers - list log drivers\n"
+ "log filter-list [OPTIONS] - list all filters for a log driver\n"
+ "\t-d <driver> - Specify the log driver to list filters from; defaults\n"
+ "\t              to console\n"
+ "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 less than or equal to <level>;\n"
+ "\t             mutually-exclusive with -L\n"
+ "\t-L <level> - Match log levels greather than or equal to <level>;\n"
+ "\t             mutually-exclusive with -l\n"
+ "\t-p - Print the filter number on success\n"
+ "log filter-remove [OPTIONS] [<num>] - Remove filter number <num>\n"
+ "\t-a - Remove ALL filters\n"
+ "\t-d <driver> - Specify the log driver to remove the filter from;\n"
+ "\t              defaults to console\n"
  "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"
@@ -179,6 +415,10 @@ U_BOOT_CMD_WITH_SUBCMDS(log, "log system", log_help_text,
  U_BOOT_SUBCMD_MKENT(level, 2, 1, do_log_level),
  U_BOOT_SUBCMD_MKENT(categories, 1, 1, do_log_categories),
  U_BOOT_SUBCMD_MKENT(drivers, 1, 1, do_log_drivers),
+ 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),
 #if CONFIG_IS_ENABLED(LOG_TEST)
--
2.28.0

123