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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPLW+4==u6Lpi-tRpGCFjuCBUARsarJx=Lg2QVAbvXX7hOyRVg@mail.gmail.com>
Date:   Wed, 13 Oct 2021 14:35:48 +0300
From:   Sam Protsenko <semen.protsenko@...aro.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Sylwester Nawrocki <s.nawrocki@...sung.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Mike Tipton <mdtipton@...eaurora.org>,
        Andy Shevchenko <andy@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Fabio Estevam <festevam@...il.com>,
        linux-clk <linux-clk@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5] clk: Add write operation for clk_parent debugfs node

On Tue, 12 Oct 2021 at 21:55, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> On Thu, Oct 07, 2021 at 09:21:58PM +0300, Sam Protsenko wrote:
> > Useful for testing mux clocks. One can write the index of the parent to
> > be set into clk_parent node, starting from 0. Example
> >
> >     # cd /sys/kernel/debug/clk/mout_peri_bus
> >     # cat clk_possible_parents
> >       dout_shared0_div4 dout_shared1_div4
> >     # cat clk_parent
> >       dout_shared0_div4
> >     # echo 1 > clk_parent
> >     # cat clk_parent
> >       dout_shared1_div4
> >
> > CLOCK_ALLOW_WRITE_DEBUGFS has to be defined in drivers/clk/clk.c in
> > order to use this feature.
>
> ...
>
> > +#ifdef CLOCK_ALLOW_WRITE_DEBUGFS
> > +     if (core->num_parents > 1)
> > +             debugfs_create_file("clk_parent", 0644, root, core,
> > +                                 &current_parent_rw_fops);
> > +     else
> > +#endif
>
> > +     {
> > +             if (core->num_parents > 0)
> > +                     debugfs_create_file("clk_parent", 0444, root, core,
> > +                                         &current_parent_fops);
> > +     }
>
> Currently there is no need to add the {} along with increased indentation
> level. I.o.w. the 'else if' is valid in C.
>

Without those {} we have two bad options:

  1. When putting subsequent 'if' block on the same indentation level
as 'else': looks ok-ish for my taste (though inconsistent with #ifdef
code) and checkpatch swears:

        WARNING: suspect code indent for conditional statements (8, 8)
        #82: FILE: drivers/clk/clk.c:3334:
        +    else
        [...]
             if (core->num_parents > 0)

  2. When adding 1 additional indentation level for subsequent 'if'
block: looks plain ugly to me, inconsistent for the case when
CLOCK_ALLOW_WRITE_DEBUGFS is not defined, but checkpatch is happy

I still think that the way I did that (with curly braces) is better
one: it's consistent for all cases, looking ok, checkpatch is happy
too. But isn't it hairsplitting? This particular case is not described
in kernel coding style doc, so it's about personal preferences.

If it's still important to you -- please provide exact code snippet
here (with indentations) for what you desire, I'll send v6. But
frankly I'd rather spend my time on something more useful. This is
minor patch, and I don't see any maintainers wishing to pull it yet.
Btw, if you know someone who can take it, could you please reference
them here?

> --
> With Best Regards,
> Andy Shevchenko
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ