linux-kernel - Re: [PATCH v2 07/24] dyndbg: fix a BUG_ON in ddebug_describe

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: <CAJfuBxxdoZ1T9nLi-X2g=zk8poLX-CtRAA=0A-i1KPq2_RhX9Q@mail.gmail.com>
Date:   Mon, 15 Jun 2020 15:53:45 -0600
From:   jim.cromie@...il.com
To:     Petr Mladek <pmladek@...e.com>
Cc:     Jason Baron <jbaron@...mai.com>,
        LKML <linux-kernel@...r.kernel.org>, akpm@...uxfoundation.org,
        Greg KH <gregkh@...uxfoundation.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH v2 07/24] dyndbg: fix a BUG_ON in ddebug_describe_flags

On Mon, Jun 15, 2020 at 7:20 AM Petr Mladek <pmladek@...e.com> wrote:
>
> On Sat 2020-06-13 09:57:21, Jim Cromie wrote:

> In all patches is missing:
>
> Signed-off-by: Jim Cromie <jim.cromie@...il.com>

right, I missed the -s invoking format-patch, v3 will have them


>
> > ---
> >  lib/dynamic_debug.c | 31 +++++++++++++++----------------
> >  1 file changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> > index 9b2445507988..aaace13d7536 100644
> > --- a/lib/dynamic_debug.c
> > +++ b/lib/dynamic_debug.c
> > @@ -87,22 +87,22 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
> >       { _DPRINTK_FLAGS_NONE, '_' },
> >  };
> >
> > +struct flagsbuf { char buf[ARRAY_SIZE(opt_array)+1]; };
>
> This looks too complicated. What about?
>
>         typedef char flags_buf[ARRAY_SIZE(opt_array) + 1];
>     used as
>         flags_buf fb;
>

I used the struct to give type safety.
old code passed a pointer to a string, and hoped it was big enough.
then used a BUG_ON to insist.
passing (the address of the) struct means the contained string is
known big enough.
and the addy is also that of the string itself (member offset 0), no overhead.

>
>         #define FLAGS_BUF_SIZE (ARRAY_SIZE(opt_array) + 1)
>     used as
>         char flags_buf[FLAGS_BUF_SIZE];
>

I never needed that constant, cuz the string is filled once,
in the function just below the struct def, using the same expression (sans +1)

I would/will update the 1-line comment on ddebug_describe_flags
and add another on the struct itself, once I figure out how to say
all of this succinctly and clearly enough.
Im open to suggestions.

thanks
jimc

>
> Best Regards,
> Petr