linux-kernel - Re: [PATCH 2/2] Staging: media: atomisp: style corrections

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHp75Vc_S7XAbHT+2secR1Re1ewkdX1d6YdYSN6UdvsoVkLViw@mail.gmail.com>
Date: Sun, 27 Apr 2025 21:49:46 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Thomas Andreatta <thomasandreatta2000@...il.com>
Cc: andy@...nel.org, linux-kernel@...r.kernel.org, linux-media@...r.kernel.org, 
	thomas.andreatta2000@...il.com
Subject: Re: [PATCH 2/2] Staging: media: atomisp: style corrections

On Sun, Apr 27, 2025 at 5:24 PM Thomas Andreatta
<thomasandreatta2000@...il.com> wrote:
> On Sun, Apr 27, 2025 at 12:41:34PM +0300, Andy Shevchenko wrote:
> > On Sat, Apr 26, 2025 at 11:15 PM Thomas Andreatta
> > <thomasandreatta2000@...il.com> wrote:

> > > Corrected consistent spacing around '*' and braces positions
> >
> > Missed period.
> > And what is the correct spacing and why?
>
> I agree that the spacing looks weird and I questioned it too, but the script
> checkpatch.pl highlights as error:
> `sh_css.c:336: ERROR: need consistent spacing around '*' (ctx:WxV)`
> `sh_css.c:338: ERROR: need consistent spacing around '*' (ctx:WxV)`
>
> Should this be ignored because the script tries its best and it becomes common
> sense that the suggested spacing is "wrong"?

checkpatch sometimes gives false positives. Need to use common sense.

...

> > >  static unsigned int get_crop_lines_for_bayer_order(const struct
> > > -       ia_css_stream_config *config);
> > > +       ia_css_stream_config * config);
> > >  static unsigned int get_crop_columns_for_bayer_order(const struct
> > > -       ia_css_stream_config *config);
> > > +       ia_css_stream_config * config);
> >
> > No, this makes it the opposite. Please, read Coding Style if it sheds
> > a light on this. In any case the kernel style is to avoid spacing
> > between asterisk and name.
>
> Understood. I'll resubmit with the correct spacing.

Looking again at the above I think the best formatting should be like this:

static unsigned int
get_crop_columns_for_bayer_order(const struct ia_css_stream_config *config);

But looking even closer it seems that these are forward declarations
for the internal functions. The question here is can we rearrange the
functions that we can remove these forward declarations completely?

-- 
With Best Regards,
Andy Shevchenko