Re: [PATCH] commands: dmesg: clear logbuffer fully if not requested otherwise

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 10/17/25 1:04 PM, Jonas Rebmann wrote:
> Previously, `dmesg -c` would clear the logbuffer except for the last 10
> lines. This comes with a fair amount of surprise given linux `dmesg -c`
> clears the whole logbuffer.
> 
> Clear the complete logbuffer given `dmesg -c` but with an optional
> argument `dmesg -c [<num>]` to keep <num> lines.
> 
> While at it, move the deleteion of log lines to the log_print function

deletion

> to prevent a corner case of lost log message if they arrive between the
> log_print() and log_clean() in dmesg.
> 
> If loglevels are selected using -l or -p in addition to cleaning being
> selected using -c, only clean those messages shown by the loglevel
> selection.

Given that this might break test suites, I think it warrants a short
addition to migration-2025.11.0.rst (create if needed).

> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  commands/dmesg.c        | 17 ++++++++++-------
>  common/console_common.c | 20 +++++++++++++++++---
>  include/linux/printk.h  |  2 +-
>  3 files changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/commands/dmesg.c b/commands/dmesg.c
> index a93ad5b359..6853f3afc9 100644
> --- a/commands/dmesg.c
> +++ b/commands/dmesg.c
> @@ -77,14 +77,20 @@ static unsigned dmesg_get_levels(const char *__args)
>  static int do_dmesg(int argc, char *argv[])
>  {
>  	int opt, ret, i;
> -	int delete_buf = 0, emit = 0;
> +	int delete_buf = -1, emit = 0;

just make it unsigned and use UINT_MAX?

delete_buf is no longer a good name for the variable IMO.

>  	unsigned flags = 0, levels = 0;
>  	char *set = NULL;
>  
> -	while ((opt = getopt(argc, argv, "ctderl:p:n:")) > 0) {
> +	while ((opt = getopt(argc, argv, "c::tderl:p:n:")) > 0) {
>  		switch (opt) {
>  		case 'c':
> -			delete_buf = 1;
> +			if (optarg) {
> +				ret = kstrtoint(optarg, 10, &delete_buf);

With type changed above, kstrtouint

> +				if (ret || delete_buf < 0)

... which simplifies this condition.

> +					return COMMAND_ERROR_USAGE;
> +			} else {
> +				delete_buf = 0;

You can initialize this right after case '0': and drop the else.

> +			}
>  			break;
>  		case 't':
>  			flags |= BAREBOX_LOG_PRINT_TIME;
> @@ -155,13 +161,10 @@ static int do_dmesg(int argc, char *argv[])
>  		return 0;
>  	}
>  
> -	ret = log_print(flags, levels);
> +	ret = log_print(flags, levels, delete_buf);

Not sure, we want to overload log_print with this.
How about we switch the ctrlc() to ctrlc_non_interruptible() and then we
are certain that there will be no messages coming in between as there
will be no resched() points (barebox multitasking is completely
cooperative).

> -int log_print(unsigned flags, unsigned levels)
> +/**

Thanks for adding docs :)

> +		if (delete_buf >= 0 && barebox_logbuf_num_messages > delete_buf)

If delete_buf were unsigned, it would simplify the condition.
The variable name is suboptimal though, something with keep in it would
be better.


Cheers,
Ahmad

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |