Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] rv: undocumented API change making da_monitor_sync_hook overridable --
commit 4c23c506287e928a0c126474d4bfba46c2216f6b Author: Wen Yang <[email protected]> rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable This commit adds #ifndef guards around da_monitor_reset_hook and EVENT_NONE_LBL in ha_monitor.h. This allows HA-based monitors to override these definitions with their own implementations before including the header, such as cancelling per-task hrtimers on teardown. > diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h > index 99b60d0d26b6d..b8c74d4d40649 100644 > --- a/include/rv/ha_monitor.h > +++ b/include/rv/ha_monitor.h > @@ -38,8 +38,14 @@ static bool ha_monitor_handle_constraint(struct da_monitor > *da_mon, > da_id_type id); > #define da_monitor_event_hook ha_monitor_handle_constraint > #define da_monitor_init_hook ha_monitor_init_env > + > +/* Overrides must still call ha_monitor_reset_env() or cancel the timer. */ > +#ifndef da_monitor_reset_hook > #define da_monitor_reset_hook ha_monitor_reset_env > +#endif > +#ifndef da_monitor_sync_hook > #define da_monitor_sync_hook() synchronize_rcu() > +#endif [Severity: Medium] The commit message explicitly claims to "Make da_monitor_reset_hook and EVENT_NONE_LBL overridable" and mentions wrapping "both definitions". However, this change also wraps a third macro, da_monitor_sync_hook, making it overridable as well. Was this third API change intentional? If so, could the commit message be updated to mention it? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
