Hi Gabriele,
On Wed, Mar 12, 2025 at 08:58:56AM +0100, Gabriele Monaco wrote:
> On Tue, 2025-03-11 at 18:05 +0100, Nam Cao wrote:
> > diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> > index 510c88bfabd4..c55d45544a16 100644
> > --- a/include/rv/da_monitor.h
> > +++ b/include/rv/da_monitor.h
> > @@ -16,58 +16,11 @@
> > #include <linux/bug.h>
> > #include <linux/sched.h>
> >
> > -#ifdef CONFIG_RV_REACTORS
> > -
> > -#define DECLARE_RV_REACTING_HELPERS(name,
> > type) \
> > -static char
> > REACT_MSG_##name[1024];
> > \
> > -
> >
> > \
> > -static inline char *format_react_msg_##name(type curr_state, type
> > event) \
> > -
> > {
> > \
> > - snprintf(REACT_MSG_##name,
> > 1024, \
> > - "rv: monitor %s does not allow event %s on state
> > %s\n", \
> > -
> > #name,
> > \
> > -
> > model_get_event_name_##name(event),
> > \
> > -
> > model_get_state_name_##name(curr_state));
> > \
> > - return
> > REACT_MSG_##name;
> > \
> > -
> > }
> > \
> > -
> >
> > \
> > -static void cond_react_##name(char
> > *msg) \
> > -
> > {
> > \
> > - if
> > (rv_##name.react)
> > \
> > -
> > rv_##name.react(msg);
> > \
> > -
> > }
> > \
> > -
> >
> > \
> > -static bool
> > rv_reacting_on_##name(void)
> > \
> > -
> > {
> > \
> > - return
> > rv_reacting_on();
> > \
> > -}
> > -
> > -#else /* CONFIG_RV_REACTOR */
> > -
> > -#define DECLARE_RV_REACTING_HELPERS(name,
> > type) \
> > -static inline char *format_react_msg_##name(type curr_state, type
> > event) \
> > -
> > {
> > \
> > - return
> > NULL;
> > \
> > -
> > }
> > \
> > -
> >
> > \
> > -static void cond_react_##name(char
> > *msg) \
> > -
> > {
> > \
> > -
> > return;
> > \
> > -
> > }
> > \
> > -
> >
> > \
> > -static bool
> > rv_reacting_on_##name(void)
> > \
> > -
> > {
> > \
> > - return
> > 0;
> > \
> > -}
> > -#endif
> > -
>
> I don't think you need to remove those helper functions, why not just
> having format_react_msg_ prepare the arguments for react?
I'm not sure what you mean. Making format_react_msg_* a macro that is
preprocessed into arguments? Then make cond_react_*() a variadic function,
so that we can "pass" format_react_msg_* to it?
Going that way would also need a vreact() variant, as cond_react_*() cannot
pass on the variadic arguments to react().
Instead, is it cleaner to do the below?
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 510c88bfabd4..e185ebf894a4 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -19,22 +19,14 @@
#ifdef CONFIG_RV_REACTORS
#define DECLARE_RV_REACTING_HELPERS(name, type)
\
-static char REACT_MSG_##name[1024];
\
-
\
-static inline char *format_react_msg_##name(type curr_state, type event)
\
-{
\
- snprintf(REACT_MSG_##name, 1024,
\
- "rv: monitor %s does not allow event %s on state %s\n",
\
- #name,
\
- model_get_event_name_##name(event),
\
- model_get_state_name_##name(curr_state));
\
- return REACT_MSG_##name;
\
-}
\
-
\
-static void cond_react_##name(char *msg)
\
+static void cond_react_##name(type curr_state, type event)
\
{
\
- if (rv_##name.react)
\
- rv_##name.react(msg);
\
+ if (!rv_##name.react)
\
+ return;
\
+ rv_##name.react("rv: monitor %s does not allow event %s on state %s\n",
\
+ #name,
\
+ model_get_event_name_##name(event),
\
+ model_get_state_name_##name(curr_state));
\
}
\
\
static bool rv_reacting_on_##name(void)
\
@@ -45,12 +37,7 @@ static bool rv_reacting_on_##name(void)
\
#else /* CONFIG_RV_REACTOR */
#define DECLARE_RV_REACTING_HELPERS(name, type)
\
-static inline char *format_react_msg_##name(type curr_state, type event)
\
-{
\
- return NULL;
\
-}
\
-
\
-static void cond_react_##name(char *msg)
\
+static void cond_react_##name(type curr_state, type event)
\
{
\
return;
\
}
\
@@ -171,7 +158,7 @@ da_event_##name(struct da_monitor *da_mon, enum
events_##name event) \
}
\
\
if (rv_reacting_on_##name())
\
- cond_react_##name(format_react_msg_##name(curr_state, event));
\
+ cond_react_##name(curr_state, event);
\
\
trace_error_##name(model_get_state_name_##name(curr_state),
\
model_get_event_name_##name(event));
\
@@ -203,7 +190,7 @@ static inline bool da_event_##name(struct da_monitor
*da_mon, struct task_struct
}
\
\
if (rv_reacting_on_##name())
\
- cond_react_##name(format_react_msg_##name(curr_state, event));
\
+ cond_react_##name(curr_state, event);
\
\
trace_error_##name(tsk->pid,
\
model_get_state_name_##name(curr_state),
\
Best regards,
Nam
