linux-kernel - Re: [PATCH] of: replace of_match_node() macro by a function when !CONFIG

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: <CAL_JsqLBmpEQVgZ1UciAdxdiSj6Ly4bpYtYPvazr9m=vRj7qEQ@mail.gmail.com>
Date: Mon, 8 Jul 2024 16:24:03 -0600
From: Rob Herring <robh@...nel.org>
To: Théo Lebrun <theo.lebrun@...tlin.com>
Cc: Saravana Kannan <saravanak@...gle.com>, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>, 
	Grégory Clement <gregory.clement@...tlin.com>, 
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>, Tawfik Bayouk <tawfik.bayouk@...ileye.com>
Subject: Re: [PATCH] of: replace of_match_node() macro by a function when !CONFIG_OF

On Mon, Jul 8, 2024 at 2:55 AM Théo Lebrun <theo.lebrun@...tlin.com> wrote:
>
> In the !CONFIG_OF case, replace the of_match_node() macro implementation
> by a static function. This ensures drivers calling of_match_node() can
> be COMPILE_TESTed.
>
> include/linux/of.h declares of_match_node() like this:
>
>         #ifdef CONFIG_OF
>         extern const struct of_device_id *of_match_node(
>                 const struct of_device_id *matches, const struct device_node *node);
>         #else
>         #define of_match_node(_matches, _node)  NULL
>         #endif
>
> When used inside an expression, those two implementations behave truly
> differently. The macro implementation has (at least) two pitfalls:
>
>  - Arguments are removed by the preprocessor meaning they do not appear
>    to the compiler. This can give "defined but not used" warnings.

It also means the arguments don't have to be defined at all which is
the reasoning the commit adding the macro gave:

    I have chosen to use a macro instead of a function to
    be able to avoid defining the first parameter.
    In fact, this "struct of_device_id *" first parameter
    is usualy not defined as well on non-dt builds.

We could change our mind here, but I suspect applying this would
result in some build failures.

>  - The returned value type is (void *)
>    versus (const struct of_device_id *).
>    It works okay if the value is stored in a variable, thanks to C's
>    implicit void pointer casting rules. It causes build errors if used
>    like `of_match_data(...)->data`.

Really, the only places of_match_node() should be used are ones
without a struct device. Otherwise, of_device_get_match_data() or
device_get_match_data() should be used instead.

Rob