linux-kernel - Re: [PATCH v2] Documentation: KUnit: Update the instructions on how to test static functions

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: <bfcc2166-5655-4733-8252-aaac84b5d6e5@riseup.net>
Date: Tue, 9 Jan 2024 13:35:34 -0300
From: Arthur Grillo <arthurgrillo@...eup.net>
To: David Gow <davidgow@...gle.com>
Cc: Brendan Higgins <brendan.higgins@...ux.dev>,
 Jonathan Corbet <corbet@....net>, linux-kselftest@...r.kernel.org,
 kunit-dev@...glegroups.com, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] Documentation: KUnit: Update the instructions on how
 to test static functions



On 09/01/24 02:44, David Gow wrote:
> On Tue, 9 Jan 2024 at 04:24, Arthur Grillo <arthurgrillo@...eup.net> wrote:
>>
>> Now that we have the VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT macros,
>> update the instructions to stop recommending including .c files.
>>
>> Signed-off-by: Arthur Grillo <arthurgrillo@...eup.net>
>> ---
>> Changes in v2:
>> - Fix #if condition
>> - Link to v1: https://lore.kernel.org/r/20240108-kunit-doc-export-v1-1-119368df0d96@riseup.net
>> ---
> 
> Thanks very much: I think we definitely should be recommending
> VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT more.
> 
> I do wonder, though, whether we should also keep the conditional
> ``#include`` example. There are some tests already using it, and it
> can be more convenient than exporting lots of symbols in some cases. I
> still think we should encourage the
> VISIBLE_IF_KUNIT/EXPORT_SYMBOL_IF_KUNIT features more, but maybe we
> leave the existing documentation there underneath. (e.g.
> "Alternatively, we can conditionally…")

I agree that, in some cases, the include way can be convenient. So, if
it's not discouraged/deprecated, I think it's better to keep the old
way.

I sent this patch because of a comment in a patch that I sent[1]. That
was when I discovered these macros and noticed the absence of
documentation on them.

[1]: https://lore.kernel.org/all/5z66ivuhfrzrnuzt6lwjfm5fuozxlgqsco3qb5rfzyf6mil5ms@2svqtlcncyjj/

~Arthur Grillo

> 
> Otherwise, this looks good, and if people think that we should avoid
> recommending the conditional-#include method (which _is_ ugly), then
> I'm happy to accept this as-is.
> 
> Thoughts?
> 
> -- David