linux-kernel - Re: [PATCH v2] ARM: decompressor: simplify libfdt builds

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: <CAK7LNATwurOd7PCra4P3X9Gsh0x43p+8iWYiw1WBnLZ7SnKiiQ@mail.gmail.com>
Date:   Mon, 20 Jan 2020 13:28:59 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] ARM: decompressor: simplify libfdt builds

Hi Russell,

On Mon, Jan 20, 2020 at 7:48 AM Russell King - ARM Linux admin
<linux@...linux.org.uk> wrote:
>
> On Sun, Jan 19, 2020 at 10:08:22AM +0900, Masahiro Yamada wrote:
> > Copying source files during the build time may not end up with
> > as clean code as expected.
> >
> > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > nicely. Let's follow that approach for the arm decompressor, too.
> >
> > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > the Makefile messes. Another nice thing is we no longer need to
> > maintain the own libfdt_env.h because the decompressor can include
> > <linux/libfdt_env.h>.
>
> Hi,
>
> This is a nice idea, but as Stephen's build has found, it is a very
> fragile change, particularly if you're doing a rebuild of an existing
> tree.
>
> Stephen's issue appears to be that - he has stale "shipped" copies
> that the old Makefile implementation created, which were attempted
> to be built with this patch applied.  The result of that is we
> try and pick up scripts/dtc/libfdt/libfdt_env.h.
>
> The whole point of the kernel build system is so that we can make
> changes to the kernel tree, and then build the kernel, and have the
> build system work out how to rebuild the kernel in a proper and safe
> way without us having to endlessly clean the build tree just because
> a few patches have been added.  This patch breaks that expectation.
>
> At the very least, this build-breaking nature needs to be mentioned,
> preferably telling people what they should be doing to fix the issue.
>
> An even better would be to find some way to avoid the issue in the
> first place, or find some way to warn about it - maybe by leaving a
> libfdt_env.h behind that has an appropriate #warning in it telling
> people what to do.  Or something.
>
> Thanks.


Sorry for causing a trouble again.


I will add the following code in arch/arm/boot/compressed/Makefile:

# These were previously generated C files. When you are building the kernel
# with O=, make sure to remove the stale files in the output tree. Otherwise,
# the build system would wrongly compile the stale ones.
ifdef building_out_of_srctree
$(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
endif



Maybe we can remove this code in the future,
but we should keep it long enough.
If the out-of-tree "git bisect" crosses this commit,
the same build error would happen.
People usually do not clean the tree while git-bisecting.



I explained it in the commit description too.



I tested the out-of-tree build
with this patch applied/dropped.


For in-tree build,
"git checkout" seems to nicely overwrite
the stale generated files with the new
check-in files.


I attached the v3 patch.

If it is OK with you,
I will put it in the patch tracker.


Thank you.



-- 
Best Regards
Masahiro Yamada

View attachment "0001-ARM-decompressor-simplify-libfdt-builds.patch" of type "text/x-patch" (7470 bytes)