Re: [PATCH v2] tracing: Use seq_buf for string concatenation

On 30/6/2569 BE 01:39, Steven Rostedt wrote:
> On Mon, 22 Jun 2026 16:46:23 +0700
> Woradorn Laodhanadhaworn <[email protected]> wrote:
> 
>>  
>>  #include <trace/events/sched.h>
>>  #include <trace/syscall.h>
>> @@ -4500,14 +4501,20 @@ static void __add_event_to_tracers(struct trace_event_call *call)
>>  extern struct trace_event_call *__start_ftrace_events[];
>>  extern struct trace_event_call *__stop_ftrace_events[];
>>  
>> -static char bootup_event_buf[COMMAND_LINE_SIZE] __initdata;
> 
> Keep the above string and just assign it.
> 
>> +static struct seq_buf bootup_event_buf __initdata = {
>> +	.buffer = (char[COMMAND_LINE_SIZE]) {},
>> +	.size = COMMAND_LINE_SIZE,
>> +};
> 
> static struct seq_buf bootup_event_seq __initdata = {
> 	.buffer = bootup_event_buf;
> 	.size = sizeof(bootup_event_buf);
> };
> 
>>  
>>  static __init int setup_trace_event(char *str)
>>  {
>> -	if (bootup_event_buf[0] != '\0')
>> -		strlcat(bootup_event_buf, ",", COMMAND_LINE_SIZE);
>> +	if (seq_buf_used(&bootup_event_buf) > 0)
>> +		seq_buf_puts(&bootup_event_buf, ",");
>> +
>> +	seq_buf_puts(&bootup_event_buf, str);
>>  
>> -	strlcat(bootup_event_buf, str, COMMAND_LINE_SIZE);
>> +	if (seq_buf_has_overflowed(&bootup_event_buf))
>> +		return -ENOMEM;
>>  
>>  	trace_set_ring_buffer_expanded(NULL);
>>  	disable_tracing_selftest("running event tracing");
>> @@ -4766,7 +4773,7 @@ static __init int event_trace_enable(void)
>>  	 */
>>  	__trace_early_add_events(tr);
>>  
>> -	early_enable_events(tr, bootup_event_buf, false);
>> +	early_enable_events(tr, (char *)seq_buf_str(&bootup_event_buf), false);
> 
> The above then would be:
> 
> 	seq_buf_str(&bootup_event_seq);
> 	early_enable_events(tr, bootup_event_buf, false);
> 
> Don't typecast a const char* to non const.
> 
>>  
>>  	trace_printk_start_comm();
>>  
>> @@ -4794,7 +4801,7 @@ static __init int event_trace_enable_again(void)
>>  	if (!tr)
>>  		return -ENODEV;
>>  
>> -	early_enable_events(tr, bootup_event_buf, true);
>> +	early_enable_events(tr, (char *)seq_buf_str(&bootup_event_buf), true);
> 
> Same here.
> 
> -- Steve
> 
>>  
>>  	return 0;
>>  }
> 

Thank you, Steven, for your review. I've sent v4:
https://lore.kernel.org/all/[email protected]

Thanks,
Woradorn