linux-kernel - Re: [PATCH 4/5] Replace timeconst.bc with mktimeconst.c

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: <87ttzemv8i.ffs@tglx>
Date:   Wed, 22 Feb 2023 00:53:01 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Rob Landley <rob@...dley.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc:     Masahiro Yamada <masahiroy@...nel.org>,
        Nicolas Schier <nicolas@...sle.eu>
Subject: Re: [PATCH 4/5] Replace timeconst.bc with mktimeconst.c

Rob!

On Tue, Feb 21 2023 at 15:00, Rob Landley wrote:

See my previous comment about Documentation. This time you even failed
to CC the maintainer of this code.

> Update of my decade-old patch replacing timeconst.pl with mktimeconst.c,
> still removing the only user of "bc" from the build.

How is 'decade-old patch' relevant changelog information?

> All that's changed since the 2015 version at:
> https://github.com/landley/aboriginal/blob/master/sources/patches/linux-noperl-timeconst.patch

That's neither interesting.

> Is one extra iteration of the loop for nanoseconds and different
> makefile plumbing calling it. In theory this could calculate the values
> at runtime in a small _init function and eliminate the header or even
> allow HZ to change at runtime.

In theory we can also build a compiler into the decompressor which
compiles and bootstraps the kernel itself, right?

> See https://lkml.iu.edu/hypermail/linux/kernel/2211.0/02589.html

I haven't seen a changelog in a while, which provides so much useless
information and lacks any content which justifies and documents the
proposed change.

>  Kbuild                    |   7 ++-
>  kernel/time/mktimeconst.c | 111 ++++++++++++++++++++++++++++++++++++
>  kernel/time/timeconst.bc  | 117 --------------------------------------

Clearly you provided evidence that the output of both tools is identical
and because you decided to reorder the defines it's not even verifyable
with diff.

But I don't even need diff to figure out that the results are not
identical:

# grep -c '#define' orig.h
25

# grep -c '#define' patched.h
31

Which means this adds 6 more unused defines to the already 8 unused
defines of today.

You clearly spent a lot of time to make this palatable to the people who
are responsible for this code.

That said, I'm not completely opposed to get rid of the bc dependency,
but if you want to sell this, then follow the documented process and
present it in a form which is acceptable.

Thanks,

        tglx