linux-kernel - Re: [PATCH v13 02/12] livepatch: Helper macros to define livepatch structures

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]

Date:   Wed, 17 Oct 2018 13:17:56 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Jiri Kosina <jikos@...nel.org>, Miroslav Benes <mbenes@...e.cz>,
        Jason Baron <jbaron@...mai.com>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Evgenii Shatokhin <eshatokhin@...tuozzo.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v13 02/12] livepatch: Helper macros to define livepatch
 structures

On Mon, Oct 15, 2018 at 02:37:03PM +0200, Petr Mladek wrote:
> The definition of struct klp_func might be a bit confusing.
> The original function is defined by name as a string.
> The new function is defined by name as a function pointer
> casted to unsigned long.
> 
> This patch adds helper macros that hide the different types.
> The functions are defined just by the name. For example:

On one hand, these macros are kind of nice, because they do the function
pointer casting for the user.

On the other hand, they hide the field names, which hurts readability a
bit.  For example, it would be easy to accidentally assign the wrong
callback function.

Also, it's unfortunate that these macros are needed in the first place.

What if we just change new_addr (and old_addr) to be 'void *'?  Then the
macros wouldn't be as useful, and we could just get rid of them.

-- 
Josh