linux-kernel - Re: init_disassemble_info() signature changes causes compile failures

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: <20220622231624.t63bkmkzphqvh3kx@alap3.anarazel.de>
Date:   Wed, 22 Jun 2022 16:16:24 -0700
From:   Andres Freund <andres@...razel.de>
To:     Quentin Monnet <quentin@...valent.com>
Cc:     open list <linux-kernel@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Andrew Burgess <aburgess@...hat.com>
Subject: Re: init_disassemble_info() signature changes causes compile failures

Hi,

On 2022-06-22 23:53:58 +0100, Quentin Monnet wrote:
> Too bad the libbfd API is changing again :/

Yea, not great. Particularly odd that
/* For compatibility with existing code.  */
#define INIT_DISASSEMBLE_INFO(INFO, STREAM, FPRINTF_FUNC, FPRINTF_STYLED_FUNC)  \

was changed. Leaving the "For compatibility with existing code." around,
despite obviously not providing compatibility...

CCed the author of that commit, maybe worth fixing?

Given that disassemble_set_printf() was added, it seems like it'd have been
easy to not change init_disassemble_info() / INIT_DISASSEMBLE_INFO() and
require disassemble_set_printf() to be called to get styled printf support.


> > The fix is easy enough, add a wrapper around fprintf() that conforms to the
> > new signature.
> >
> > However I assume the necessary feature test and wrapper should only be added
> > once? I don't know the kernel stuff well enough to choose the right structure
> > here.
> 
> We can probably find a common header for the wrapper under
> tools/include/. One possibility could be a new header under
> tools/include/tools/, like for libc_compat.h. Although personally I
> don't mind too much about redefining the wrapper several times given
> how short it is, and because maybe some tools could redefine it anyway
> to use colour output in the future.

I'm more bothered by duplicating the necessary ifdefery than duplicating the
short fprintf wrapper...


> The feature test would better be shared, it would probably be similar
> to what was done in the following commit to accommodate for a previous
> change in libbfd:
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fb982666e380c1632a74495b68b3c33a66e76430

Ah, beautiful hand-rolled feature tests :)


> > Attached is my local fix for perf. Obviously would need work to be a real
> > solution.
> 
> Thanks a lot! Would you be willing to submit a patch for the feature
> detection and wrapper?

I'll give it a go, albeit probably not today.

Greetings,

Andres Freund