[PATCH] kunit: Fix kernel-doc for EXPORT_SYMBOL_IF_KUNIT

[Michal Wajdeczko]

While kunit/visibility.h is today not included in any generated
kernel documentation, also likely due to the fact that none of the
existing comments are correctly recognized as kernel-doc, but once
we decide to add this header and fix the tool, there will be:

../include/kunit/visibility.h:61: warning: Function parameter or
struct member 'symbol' not described in 'EXPORT_SYMBOL_IF_KUNIT'

Signed-off-by: Michal Wajdeczko <michal.w...@intel.com>
---
Cc: Rae Moar <rm...@google.com>
Cc: David Gow <davi...@google.com>
---
include/kunit/visibility.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/kunit/visibility.h b/include/kunit/visibility.h
index 0dfe35feeec6..efff77b58dd6 100644
--- a/include/kunit/visibility.h
+++ b/include/kunit/visibility.h
@@ -22,6 +22,7 @@
* EXPORTED_FOR_KUNIT_TESTING namespace only if CONFIG_KUNIT is
* enabled. Must use MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING)
* in test file in order to use symbols.
+ * @symbol: the symbol identifier to export
*/
#define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, \
EXPORTED_FOR_KUNIT_TESTING)
--
2.43.0

[Rae Moar]

On Thu, Aug 29, 2024 at 11:32 AM Michal Wajdeczko
<michal.w...@intel.com> wrote:
>
> While kunit/visibility.h is today not included in any generated
> kernel documentation, also likely due to the fact that none of the
> existing comments are correctly recognized as kernel-doc, but once
> we decide to add this header and fix the tool, there will be:
>
> ../include/kunit/visibility.h:61: warning: Function parameter or
> struct member 'symbol' not described in 'EXPORT_SYMBOL_IF_KUNIT'
>
> Signed-off-by: Michal Wajdeczko <michal.w...@intel.com>

Hello!

This looks good to me. Thanks for adding this. We will need to do a
patch on making this recognized as a kernel-doc at some point.

Reviewed-by: Rae Moar <rm...@google.com>

Thanks!
-Rae

> ---
> Cc: Rae Moar <rm...@google.com>
> Cc: David Gow <davi...@google.com>
> ---
> include/kunit/visibility.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/kunit/visibility.h b/include/kunit/visibility.h
> index 0dfe35feeec6..efff77b58dd6 100644
> --- a/include/kunit/visibility.h
> +++ b/include/kunit/visibility.h
> @@ -22,6 +22,7 @@
> * EXPORTED_FOR_KUNIT_TESTING namespace only if CONFIG_KUNIT is
> * enabled. Must use MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING)
> * in test file in order to use symbols.
> + * @symbol: the symbol identifier to export
> */
> #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, \
> EXPORTED_FOR_KUNIT_TESTING)
> --
> 2.43.0
>

> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20240829183150.1616-1-michal.wajdeczko%40intel.com.

[Michal Wajdeczko]

On 29.08.2024 20:58, Rae Moar wrote:
> On Thu, Aug 29, 2024 at 11:32 AM Michal Wajdeczko
> <michal.w...@intel.com> wrote:
>>
>> While kunit/visibility.h is today not included in any generated
>> kernel documentation, also likely due to the fact that none of the
>> existing comments are correctly recognized as kernel-doc, but once
>> we decide to add this header and fix the tool, there will be:
>>
>> ../include/kunit/visibility.h:61: warning: Function parameter or
>> struct member 'symbol' not described in 'EXPORT_SYMBOL_IF_KUNIT'
>>
>> Signed-off-by: Michal Wajdeczko <michal.w...@intel.com>
>
> Hello!
>
> This looks good to me. Thanks for adding this. We will need to do a
> patch on making this recognized as a kernel-doc at some point.

here it goes [1]

[1]
https://lore.kernel.org/linux-doc/20240829202529.1660...@intel.com/

[David Gow]

On Thu, 29 Aug 2024 at 11:32, Michal Wajdeczko
<michal.w...@intel.com> wrote:
>
> While kunit/visibility.h is today not included in any generated
> kernel documentation, also likely due to the fact that none of the
> existing comments are correctly recognized as kernel-doc, but once
> we decide to add this header and fix the tool, there will be:
>
> ../include/kunit/visibility.h:61: warning: Function parameter or
> struct member 'symbol' not described in 'EXPORT_SYMBOL_IF_KUNIT'
>
> Signed-off-by: Michal Wajdeczko <michal.w...@intel.com>
> ---
> Cc: Rae Moar <rm...@google.com>
> Cc: David Gow <davi...@google.com>
> ---

Acked-by: David Gow <davi...@google.com>

Thanks,
-- David

[Rae Moar]

On Thu, Aug 29, 2024 at 4:28 PM Michal Wajdeczko
<michal.w...@intel.com> wrote:
>
>
>
> On 29.08.2024 20:58, Rae Moar wrote:
> > On Thu, Aug 29, 2024 at 11:32 AM Michal Wajdeczko
> > <michal.w...@intel.com> wrote:
> >>
> >> While kunit/visibility.h is today not included in any generated
> >> kernel documentation, also likely due to the fact that none of the
> >> existing comments are correctly recognized as kernel-doc, but once
> >> we decide to add this header and fix the tool, there will be:
> >>
> >> ../include/kunit/visibility.h:61: warning: Function parameter or
> >> struct member 'symbol' not described in 'EXPORT_SYMBOL_IF_KUNIT'
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.w...@intel.com>
> >
> > Hello!
> >
> > This looks good to me. Thanks for adding this. We will need to do a
> > patch on making this recognized as a kernel-doc at some point.
>
> here it goes [1]
>
> [1]
> https://lore.kernel.org/linux-doc/20240829202529.1660...@intel.com/

Hi Michal!

Thanks for your fast response to this! It looks like this patch
changes the kernel-doc specification, which is a very worthwhile
conversation, but in this instance I intended to change the code
comments in visibility.h to match the current kernel-doc
specification. I am also happy to make those edits in a future patch.

Thanks!
-Rae

[Michal Wajdeczko]

Hi Rae,

On 06.09.2024 21:57, Rae Moar wrote:
> On Thu, Aug 29, 2024 at 4:28 PM Michal Wajdeczko
> <michal.w...@intel.com> wrote:
>>
>>
>>
>> On 29.08.2024 20:58, Rae Moar wrote:
>>> On Thu, Aug 29, 2024 at 11:32 AM Michal Wajdeczko
>>> <michal.w...@intel.com> wrote:
>>>>
>>>> While kunit/visibility.h is today not included in any generated
>>>> kernel documentation, also likely due to the fact that none of the
>>>> existing comments are correctly recognized as kernel-doc, but once
>>>> we decide to add this header and fix the tool, there will be:
>>>>
>>>> ../include/kunit/visibility.h:61: warning: Function parameter or
>>>> struct member 'symbol' not described in 'EXPORT_SYMBOL_IF_KUNIT'
>>>>
>>>> Signed-off-by: Michal Wajdeczko <michal.w...@intel.com>
>>>
>>> Hello!
>>>
>>> This looks good to me. Thanks for adding this. We will need to do a
>>> patch on making this recognized as a kernel-doc at some point.
>>
>> here it goes [1]
>>
>> [1]
>> https://lore.kernel.org/linux-doc/20240829202529.1660...@intel.com/
>
> Hi Michal!
>
> Thanks for your fast response to this! It looks like this patch
> changes the kernel-doc specification,

I'm not sure it changes anything as in the spec there is just:

"The opening comment mark ``/**`` is used for kernel-doc comments."

without any mention that it has to be at the start of the line.

In fact above patch was able to detect (too) many mistakes in existing
comments that were either incomplete or false kernel-doc, but once we
fixed those, there is high chance patch will be accepted.

Michal