Re: [PATCH v2] staging: media: atomisp: refactor pipe graph dump stage formatting

[Andy Shevchenko]

On Sat, Jul 04, 2026 at 09:39:20PM -0700, Neal Patalay wrote:
> The original implementation of ia_css_debug_pipe_graph_dump_stage()
> includes an off-by-one error where the original strscpy() size dropped
> characters immediately before newlines. It also allocates over 600
> bytes across multiple buffers on the stack. Address these shortcomings
> and reduce stack usage with a single 256 byte buffer via a new helper
> function, ia_css_debug_build_info().

...

> +static void ia_css_debug_build_info(char *info, size_t info_size,
> +				    int *offset,
> +				    const char *flag_str, size_t flag_str_size,
> +				    int *line_len)
> +{
> +	/*
> +	 * If new line length exceeds max line length,
> +	 * replace the last ',' with a "\\n"
> +	 */
> +	if (*line_len > 0 && info_size - *offset >= 2 &&
> +	    *line_len + flag_str_size > ENABLE_LINE_MAX_LENGTH) {
> +		info[*offset - 1] = '\\';
> +		info[*offset] = 'n';
> +		*offset += 1;
> +		*line_len = 0;
> +	}

> +	int len_written = scnprintf(info + *offset, info_size - *offset,
> +				    "%s,", flag_str);

The definition should go to the top of the function, there are only a few
exceptions and none is for this case.

Also for the sake of better layering, introduce local variables to track values
of offset and line_len and only assign them at the end.

> +	*offset += len_written;
> +	*line_len += len_written;

So this will become

	int len = *line_len;
	int off = *offset;
	int len_written;
	... // here use only off and len
	*offset = off + len_written;
	*line_len = len + len_written;

> +}

...

> +#define ADD_INFO(flag, flag_str)						\
> +		do {								\
> +			if (bi->enable.flag)					\
> +				ia_css_debug_build_info(enable_info,		\
> +							sizeof(enable_info),	\
> +							&offset,		\
> +							flag_str,		\
> +							sizeof(flag_str),	\
> +							&line_len);		\
> +		} while (0)

There is no need to have do {} while (0) as there is conditional already and the
macro doesn't return anything. Also it's wrongly indented.

The below should work.

#define ADD_INFO(flag, flag_str)						\
	if (bi->enable.flag)							\
		ia_css_debug_build_info(enable_info, sizeof(enable_info),	\
					&offset,				\
					flag_str, sizeof(flag_str),		\
					&line_len);				\

-- 
With Best Regards,
Andy Shevchenko