Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
MIME-Version: 1.0
References: <20231125103116.797608-1-masahiroy@kernel.org> <20231125103116.797608-3-masahiroy@kernel.org>
 <20231130003207.GD2513828@dev-arch.thelio-3990X>
In-Reply-To: <20231130003207.GD2513828@dev-arch.thelio-3990X>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Sun, 3 Dec 2023 18:03:10 +0900
Message-ID: <CAK7LNATxdJ6b1RYjOin2Ni991LU68rOd=OonRemrardTWMZ=Pg@mail.gmail.com>
Subject: Re: [PATCH 3/3] modpost: move exit(1) for fatal() to modpost.h
To:     Nathan Chancellor <nathan@kernel.org>
Cc:     linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Nicolas Schier <nicolas@fjasle.eu>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Nov 30, 2023 at 9:32=E2=80=AFAM Nathan Chancellor <nathan@kernel.or=
g> wrote:
>
> On Sat, Nov 25, 2023 at 07:31:16PM +0900, Masahiro Yamada wrote:
> > fatal() never returns, but compilers are not aware of this fact because
> > exit(1) is called within the modpost_log() definition.
> >
> > Move exit(1) to the fatal() macro so that compilers can identify
> > unreachable code flows.
> >
> > Remove the initializer for 'taddr' in section_rel(), as compilers now
> > recognize this is not an uninitialized bug.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> While I did not see any warnings when testing, it seems like this patch
> should come before patch 2 to avoid any potential problems from dropping
> the unreachable code (since the compiler won't be able to infer that
> fatal() is __noreturn without this change); doesn't really matter though
> so:


My eyes spotted 2/3, but I can swap the patch order
and rephase the patch description because clang can
detect the unreachable code.


> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>
> > ---
> >
> >  scripts/mod/modpost.c | 4 +---
> >  scripts/mod/modpost.h | 2 +-
> >  2 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 013fc5031bc7..696c583a14ec 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -84,8 +84,6 @@ void modpost_log(enum loglevel loglevel, const char *=
fmt, ...)
> >       vfprintf(stderr, fmt, arglist);
> >       va_end(arglist);
> >
> > -     if (loglevel =3D=3D LOG_FATAL)
> > -             exit(1);
> >       if (loglevel =3D=3D LOG_ERROR)
> >               error_occurred =3D true;
> >  }
> > @@ -1415,7 +1413,7 @@ static void section_rel(struct module *mod, struc=
t elf_info *elf,
> >
> >       for (rel =3D start; rel < stop; rel++) {
> >               Elf_Sym *tsym;
> > -             Elf_Addr taddr =3D 0, r_offset;
> > +             Elf_Addr taddr, r_offset;
> >               unsigned int r_type, r_sym;
> >               void *loc;
> >
> > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> > index 9fe974dc1a52..aaa67b24775e 100644
> > --- a/scripts/mod/modpost.h
> > +++ b/scripts/mod/modpost.h
> > @@ -215,4 +215,4 @@ modpost_log(enum loglevel loglevel, const char *fmt=
, ...);
> >   */
> >  #define warn(fmt, args...)   modpost_log(LOG_WARN, fmt, ##args)
> >  #define error(fmt, args...)  modpost_log(LOG_ERROR, fmt, ##args)
> > -#define fatal(fmt, args...)  modpost_log(LOG_FATAL, fmt, ##args)
> > +#define fatal(fmt, args...)  do { modpost_log(LOG_FATAL, fmt, ##args);=
 exit(1); } while (1)
> > --
> > 2.40.1
> >
>


--=20
Best Regards
Masahiro Yamada