staging: media: atomisp: fix indentation to use tabs instead of spaces

[PATCH] staging: media: atomisp: fix indentation to use tabs instead of spaces

Posted by Michael Ugrin 2 months ago

Fix whitespace issue where a continuation line used spaces
instead of tabs for indentation.

Signed-off-by: Michael Ugrin <mugrinphoto@gmail.com>
---
 .../staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
index b411ca2f415e0..966d4efb200c5 100644
--- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
+++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
@@ -1257,7 +1257,7 @@ ia_css_debug_pipe_graph_dump_stage(
 					p--;
 				/* Last comma found, copy till that comma */
 				strscpy(enable_info1, ei,
-                                        p > sizeof(enable_info1) ? sizeof(enable_info1) : p);
+					p > sizeof(enable_info1) ? sizeof(enable_info1) : p);
 
 				ei += p + 1;
 				l = strlen(ei);
-- 
2.43.0

Re: [PATCH] staging: media: atomisp: fix indentation to use tabs instead of spaces

Posted by Dan Carpenter 2 months ago

On Fri, Apr 10, 2026 at 05:55:12PM -0700, Michael Ugrin wrote:
> Fix whitespace issue where a continuation line used spaces
> instead of tabs for indentation.
> 
> Signed-off-by: Michael Ugrin <mugrinphoto@gmail.com>
> ---
>  .../staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c  | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> index b411ca2f415e0..966d4efb200c5 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> +++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> @@ -1257,7 +1257,7 @@ ia_css_debug_pipe_graph_dump_stage(
>  					p--;
>  				/* Last comma found, copy till that comma */
>  				strscpy(enable_info1, ei,
> -                                        p > sizeof(enable_info1) ? sizeof(enable_info1) : p);
> +					p > sizeof(enable_info1) ? sizeof(enable_info1) : p);

Better to use:

	strscpy(enable_info1, ei, umin(p, sizeof(enable_info1)));

Same for the other strscpy() calls as well.

regards,
dan carpenter

Re: [PATCH] staging: media: atomisp: fix indentation to use tabs instead of spaces

Posted by David Laight 2 months ago

On Sat, 11 Apr 2026 13:51:35 +0300
Dan Carpenter <error27@gmail.com> wrote:

> On Fri, Apr 10, 2026 at 05:55:12PM -0700, Michael Ugrin wrote:
> > Fix whitespace issue where a continuation line used spaces
> > instead of tabs for indentation.
> > 
> > Signed-off-by: Michael Ugrin <mugrinphoto@gmail.com>
> > ---
> >  .../staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c  | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> > index b411ca2f415e0..966d4efb200c5 100644
> > --- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> > +++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> > @@ -1257,7 +1257,7 @@ ia_css_debug_pipe_graph_dump_stage(
> >  					p--;
> >  				/* Last comma found, copy till that comma */
> >  				strscpy(enable_info1, ei,
> > -                                        p > sizeof(enable_info1) ? sizeof(enable_info1) : p);
> > +					p > sizeof(enable_info1) ? sizeof(enable_info1) : p);  
> 
> Better to use:
> 
> 	strscpy(enable_info1, ei, umin(p, sizeof(enable_info1)));
> 
> Same for the other strscpy() calls as well.

Or refactor that code so it doesn't use 600+ bytes of stack and
lots of scanning of long strings.

From a quick scan it seems to be generating a string of xxx,yyy, with the ','
replaced by '\n' to avoid anything longer than 25 characters and at most three lines.
I'm sure you could write and use a function that lets you have a lot of lines like:
	offset = add_flag(info, offset, bi->enable.reduced_pipe, "rp");

Oh, and none of your char_enable_info[] arrays are guaranteed to
be aligned either.
So all the snprint and strscpy calls into different buffers are pointless.

	David

> 
> regards,
> dan carpenter
> 
>

Re: [PATCH] staging: media: atomisp: fix indentation to use tabs instead of spaces

Posted by Michael Ugrin 2 months ago

Thanks for the review, Dan and David.

I'll work on a v2,replacing the ternary with umin() for that strscpy
call and the other strscpy calls in the function.

The larger refactor to eliminate the stack buffers and build the
output incrementally sounds like a great improvement. I'm a new
contributor and that's beyond my scope for this patch, but I'd like to
come back and tackle it as a separate effort once I'm more comfortable
with the codebase.

Thanks again for the guidance.

- Michael

On Sat, Apr 11, 2026 at 7:42 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Sat, 11 Apr 2026 13:51:35 +0300
> Dan Carpenter <error27@gmail.com> wrote:
>
> > On Fri, Apr 10, 2026 at 05:55:12PM -0700, Michael Ugrin wrote:
> > > Fix whitespace issue where a continuation line used spaces
> > > instead of tabs for indentation.
> > >
> > > Signed-off-by: Michael Ugrin <mugrinphoto@gmail.com>
> > > ---
> > >  .../staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c  | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> > > index b411ca2f415e0..966d4efb200c5 100644
> > > --- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> > > +++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> > > @@ -1257,7 +1257,7 @@ ia_css_debug_pipe_graph_dump_stage(
> > >                                     p--;
> > >                             /* Last comma found, copy till that comma */
> > >                             strscpy(enable_info1, ei,
> > > -                                        p > sizeof(enable_info1) ? sizeof(enable_info1) : p);
> > > +                                   p > sizeof(enable_info1) ? sizeof(enable_info1) : p);
> >
> > Better to use:
> >
> >       strscpy(enable_info1, ei, umin(p, sizeof(enable_info1)));
> >
> > Same for the other strscpy() calls as well.
>
> Or refactor that code so it doesn't use 600+ bytes of stack and
> lots of scanning of long strings.
>
> From a quick scan it seems to be generating a string of xxx,yyy, with the ','
> replaced by '\n' to avoid anything longer than 25 characters and at most three lines.
> I'm sure you could write and use a function that lets you have a lot of lines like:
>         offset = add_flag(info, offset, bi->enable.reduced_pipe, "rp");
>
> Oh, and none of your char_enable_info[] arrays are guaranteed to
> be aligned either.
> So all the snprint and strscpy calls into different buffers are pointless.
>
>         David
>
> >
> > regards,
> > dan carpenter
> >
> >
>

Re: [PATCH] staging: media: atomisp: fix indentation to use tabs instead of spaces

Posted by Andy Shevchenko 2 months ago

On Sat, Apr 11, 2026 at 8:14 PM Michael Ugrin <mugrinphoto@gmail.com> wrote:
>
> Thanks for the review, Dan and David.
>
> I'll work on a v2,replacing the ternary with umin() for that strscpy
> call and the other strscpy calls in the function.

Why not simply min()? Wouldn't it work (if type of p is aligned accordingly)?

Note, we prefer the (more) real patches, id est what David suggested
seems to me more useful to this driver to have in the long term.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2] staging: media: atomisp: use umin() for strscpy size arguments

Posted by Michael Ugrin 2 months ago

Replace open-coded ternary min expressions with umin() in
strscpy() calls, as suggested by Dan Carpenter.

Signed-off-by: Michael Ugrin <mugrinphoto@gmail.com>
---
 .../atomisp/pci/runtime/debug/src/ia_css_debug.c      | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
index b411ca2f415e0..60bb10dac5891 100644
--- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
+++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
@@ -1256,8 +1256,7 @@ ia_css_debug_pipe_graph_dump_stage(
 				while (ei[p] != ',')
 					p--;
 				/* Last comma found, copy till that comma */
-				strscpy(enable_info1, ei,
-                                        p > sizeof(enable_info1) ? sizeof(enable_info1) : p);
+				strscpy(enable_info1, ei, umin(p, sizeof(enable_info1)));
 
 				ei += p + 1;
 				l = strlen(ei);
@@ -1268,8 +1267,7 @@ ia_css_debug_pipe_graph_dump_stage(
 					 * it is not guaranteed dword aligned
 					 */
 
-					strscpy(enable_info2, ei,
-						l > sizeof(enable_info2) ? sizeof(enable_info2) : l);
+					strscpy(enable_info2, ei, umin(l, sizeof(enable_info2)));
 
 					snprintf(enable_info, sizeof(enable_info), "%s\\n%s",
 						 enable_info1, enable_info2);
@@ -1280,8 +1278,7 @@ ia_css_debug_pipe_graph_dump_stage(
 					while (ei[p] != ',')
 						p--;
 
-					strscpy(enable_info2, ei,
-						p > sizeof(enable_info2) ? sizeof(enable_info2) : p);
+					strscpy(enable_info2, ei, umin(p, sizeof(enable_info2)));
 
 					ei += p + 1;
 					l = strlen(ei);
@@ -1303,7 +1300,7 @@ ia_css_debug_pipe_graph_dump_stage(
 						while (ei[p] != ',')
 							p--;
 						strscpy(enable_info3, ei,
-							p > sizeof(enable_info3) ? sizeof(enable_info3) : p);
+							umin(p, sizeof(enable_info3)));
 						ei += p + 1;
 						strscpy(enable_info3, ei,
 							sizeof(enable_info3));
-- 
2.43.0

Re: [PATCH v2] staging: media: atomisp: use umin() for strscpy size arguments

Posted by Dan Carpenter 2 months ago

On Sat, Apr 11, 2026 at 10:34:05AM -0700, Michael Ugrin wrote:
> Replace open-coded ternary min expressions with umin() in
> strscpy() calls, as suggested by Dan Carpenter.
> 
> Signed-off-by: Michael Ugrin <mugrinphoto@gmail.com>
> ---

LGTM.

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter