Re: [PATCH 4/4] sim: Suppress non-literal printf warning

[Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>]

Tsukasa OI <research_trasio@irq.a4lg.com> writes:

> Clang generates a warning if the format parameter of a printf-like function
> is not a literal.  However, on hw_vabort, it's unavoidable to use non-
> literal as a format string (unless we make huge redesign).
>
> We have "include/diagnostics.h" to suppress certain warnings only when
> necessary.  Because DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL can suppress
> warnings when the format parameter of a printf-like function is not a
> literal, this commit adds this (only where necessary) to suppress this
> error with "-Werror", the default configuration.
>
> sim/ChangeLog:
>
> 	* common/sim-hw.c (hw_vabort): Suppress non-literal printf warning
> 	by using DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL.
> ---
>  sim/common/sim-hw.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/sim/common/sim-hw.c b/sim/common/sim-hw.c
> index cece5638bc9..36f355d2262 100644
> --- a/sim/common/sim-hw.c
> +++ b/sim/common/sim-hw.c
> @@ -425,10 +425,13 @@ hw_vabort (struct hw *me,
>    strcat (msg, ": ");
>    strcat (msg, fmt);
>    /* report the problem */
> +  DIAGNOSTIC_PUSH
> +  DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
>    sim_engine_vabort (hw_system (me),
>  		     STATE_HW (hw_system (me))->cpu,
>  		     STATE_HW (hw_system (me))->cia,
>  		     msg, ap);
> +  DIAGNOSTIC_POP

Rather than disabling diagnostics, I'd like to propose the patch below
which expands FMT and AP within sim-hw.c, then passes the expanded
string through to sim_engine_abort.  What do you think of this?

My motivation is to avoid disabling diagnostics as much as possible.

As far as I can tell the host_callback_struct::evprintf_filtered
callback is just the standard printf API, so using vsnprintf should
expand everything correctly.

Thanks,
Andrew

---

diff --git a/sim/common/sim-hw.c b/sim/common/sim-hw.c
index cece5638bc9..7bfe91e4ae2 100644
--- a/sim/common/sim-hw.c
+++ b/sim/common/sim-hw.c
@@ -408,8 +408,11 @@ hw_vabort (struct hw *me,
 	   const char *fmt,
 	   va_list ap)
 {
+  int len;
   const char *name;
   char *msg;
+  va_list cpy;
+
   /* find an identity */
   if (me != NULL && hw_path (me) != NULL && hw_path (me) [0] != '\0')
     name = hw_path (me);
@@ -419,16 +422,19 @@ hw_vabort (struct hw *me,
     name = hw_family (me);
   else
     name = "device";
-  /* construct an updated format string */
-  msg = alloca (strlen (name) + strlen (": ") + strlen (fmt) + 1);
-  strcpy (msg, name);
-  strcat (msg, ": ");
-  strcat (msg, fmt);
+
+  /* Expand FMT and AP into MSG buffer.  */
+  va_copy (cpy, ap);
+  len = vsnprintf (NULL, 0, fmt, cpy) + 1;
+  va_end (cpy);
+  msg = alloca (len);
+  vsnprintf (msg, len, fmt, ap);
+
   /* report the problem */
-  sim_engine_vabort (hw_system (me),
-		     STATE_HW (hw_system (me))->cpu,
-		     STATE_HW (hw_system (me))->cia,
-		     msg, ap);
+  sim_engine_abort (hw_system (me),
+		    STATE_HW (hw_system (me))->cpu,
+		    STATE_HW (hw_system (me))->cia,
+		    "%s: %s", name, msg);
 }
 
 void