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  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:   Fri, 23 Sep 2022 13:01:14 +0200
From:   Javier Martinez Canillas <>
To:     Thomas Zimmermann <>,
        Maxime Ripard <>,
        Jernej Skrabec <>,
        Rodrigo Vivi <>,
        Ben Skeggs <>,
        David Airlie <>,
        Maxime Ripard <>,
        Joonas Lahtinen <>,
        Emma Anholt <>,
        Karol Herbst <>,
        Samuel Holland <>,
        Jani Nikula <>,
        Daniel Vetter <>, Lyude Paul <>,
        Maarten Lankhorst <>,
        Tvrtko Ursulin <>,
        Chen-Yu Tsai <>
Cc:     Dom Cobley <>,
        Dave Stevenson <>,
        Phil Elwell <>,,,,,
        Mateusz Kwiatkowski <>,
        Hans de Goede <>,
        Noralf Trønnes <>,
        Geert Uytterhoeven <>,,
Subject: Re: [PATCH v2 13/33] drm/client: Add some tests for

On 9/23/22 12:30, Thomas Zimmermann wrote:


>>>> +
>>>> +#include "tests/drm_client_modeset_test.c"
>>>> +#endif
>>> I strongly dislike this style of including source files in each other.
>>> It's a recipe for all kind of build errors. Can you do something else?
>> This seems to be the convention used to test static functions and what's
>> documented in the Kunit docs:
> That document says " option is to conditionally #include the test 
> file...". This doesn't sound like a strong requirement.

That's true.

>> I agree with you that's not ideal but I think that consistency with how
>> it is done by other subsystems is also important.
>>> As the tested function is an internal interface, maybe export a wrapper
>>> if tests are enabled, like this:
>>> struct drm_display_mode *
>>> drm_connector_pick_cmdline_mode_kunit(drm_conenctor)
>>> {
>>>     return drm_connector_pick_cmdline_mode(connector)
>>> }
>>> EXPORT_SYMBOL(drm_connector_pick_cmdline_mode_kunit)
>>> #endif
>>> The wrapper's declaration can be located in the kunit test file.
>> But that's also not nice since we are artificially exposing these only
>> to allow the static functions to be called from unit tests. And would
>> be a different approach than the one used by all other subsystems...
> There's the problem of interference between the source files when 
> building the code. It's also not the same source code after including 
> the test file. At a minimum, including the tests' source file further 
> includes more files. <kunit/tests.h> also includes quite a few of Linux 
> header files.
> IMHO the current convention (if any) is far from optimal and we should 
> consider breaking it.

Yes, I agree with that. But probably we should be explicit about the
wrapper export symbols to access static functions pattern in the KUnit
docs so other subsystems could do the same and it becomes a convention ?
> Best regards
> Thomas

Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Powered by blists - more mailing lists