On Wed, Mar 16, 2022 at 12:53 AM Thomas Huth <thuth@xxxxxxxxxx> wrote:
>
> On 16/03/2022 08.43, Andrew Jones wrote:
> > On Tue, Mar 15, 2022 at 11:02:14PM -0700, Bill Wendling wrote:
> >> Clang warns about using a bitwise '|' with boolean operands. This seems
> >> to be due to a small typo.
> >>
> >> lib/libfdt/fdt_rw.c:438:6: warning: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> >> if (can_assume(LIBFDT_ORDER) |
> >>
> >> Using '||' removes this warnings.
> >>
> >> Signed-off-by: Bill Wendling <morbo@xxxxxxxxxx>
> >> ---
> >> lib/libfdt/fdt_rw.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/lib/libfdt/fdt_rw.c b/lib/libfdt/fdt_rw.c
> >> index 13854253ff86..3320e5559cac 100644
> >> --- a/lib/libfdt/fdt_rw.c
> >> +++ b/lib/libfdt/fdt_rw.c
> >> @@ -435,7 +435,7 @@ int fdt_open_into(const void *fdt, void *buf, int bufsize)
> >> return struct_size;
> >> }
> >>
> >> - if (can_assume(LIBFDT_ORDER) |
> >> + if (can_assume(LIBFDT_ORDER) ||
> >> !fdt_blocks_misordered_(fdt, mem_rsv_size, struct_size)) {
> >> /* no further work necessary */
> >> err = fdt_move(fdt, buf, bufsize);
> >> --
> >> 2.35.1.723.g4982287a31-goog
> >>
> >
> > This is fixed in libfdt upstream with commit 7be250b4 ("libfdt:
> > Correct condition for reordering blocks"), which is in v1.6.1.
> > We can either take this patch as a backport of 7be250b4 or we
> > can rebase all of our libfdt to v1.6.1. Based on the number of
> > fixes in v1.6.1, which appear to be mostly for compiling with
> > later compilers, I'm in favor of rebasing.
>
> +1 for updating to v1.6.1 completely.
>
Also +1. :-) Thank you!
-bw
> > Actually, we can also use this opportunity to [re]visit the
> > idea of changing libfdt to a git submodule. I'd like to hear
> > opinions on that.
>
> I'm always a little bit torn when it comes to this question. Sure, git
> submodules maybe make the update easier ... but they are a real pita when
> you're working with remote machines that might not have direct connection to
> the internet. For example, I'm used to rsync my sources to the non-x86
> machines, and if you forget to update the submodule to the right state
> before the sync, you've just lost. So from my side, it's a preference for
> continuing without submodules (but I won't insist if everybody else wants to
> have them instead).
>
> Thomas
>
