Re: [PATCH v3 14/17] verification/rvgen: Add selftests for rvgen kunit

[Wen Yang <wen.yang@linux.dev>]

On 6/29/26 15:04, Gabriele Monaco wrote:
> On Mon, 2026-06-29 at 01:06 +0800, Wen Yang wrote:
>> On 6/25/26 20:14, Gabriele Monaco wrote:
>>> +static void handle_example_event(void *data, /* XXX: fill header */)
>>> +{
>>> +	ltl_atom_update(task, LTL_EVENT_A, true/false);
>>> +}
>>> +
>>> +static int enable_test_ltl_kunit(void)
>>> +{
>>> +	int retval;
>>> +
>>> +	retval = ltl_monitor_init();
>>> +	if (retval)
>>> +		return retval;
>>> +
>>> +	rv_attach_trace_probe("test_ltl_kunit", /* XXX: tracepoint */,
>>> handle_example_event);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void disable_test_ltl_kunit(void)
>>> +{
>>> +	rv_detach_trace_probe("test_ltl_kunit", /* XXX: tracepoint */,
>>> handle_sample_event);
>>> +
>>
>> one typo:
>>          handle_sample_event should be handle_example_event.
> 
> Keep in mind that those files are not ready to build, users need to
> touch them anyway after generation from rvgen. Nevertheless, this has
> been inconsistent for a while and I should fix it.
> 

Note that the same typo (handle_sample_event) appears in the other two
LTL golden files added in patch 05/17 as well: ltl_pertask.c and
test_ltl.c. The root is likely in the rvgen LTL template itself, so
the fix there would regenerate all three consistently.


>>> +	ltl_monitor_destroy();
>>> +}
> ...
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_AUTHOR(/* TODO */);
>>
>> Please use a valid string here.
> 
> Likewise, this is not supposed to build, we are just validating what
> rvgen produces and that's the expected output, the user will need to
> fill it with an appropriate string.
> 
> LTL uses a different approach compared to DA/HA in this template, I'm not
> sure it's worth aligning the two..
> 

Understood on the intent. One small concern:  MODULE_AUTHOR(/* TODO */)
is subtly different from an XXX comment — a C preprocessor strips the
comment before macro expansion, leaving  MODULE_AUTHOR() with an empty
argument. When the user does go to compile, they get a cryptic error
rather than a clear "fill this in" hint.

--
Best wishes,
Wen