ACK: [PATCH] Add GNU printf format attribute specifier for make_message()

[Colin Ian King]

On 20/03/2020 01:47, Alex Hung wrote:
> 
> 
> On Thu, Mar 19, 2020 at 6:15 PM Colin Ian King <colin.king at canonical.com
> <mailto:colin.king at canonical.com>> wrote:
> 
>     On 19/03/2020 23:33, Alex Hung wrote:
>     > I found this breaks on 32bit (both PC & ARM) systems...
> 
>     Apologies for that. Let's just ignore (or revert) this one and I'll work
>     on a V2 for the next release.
> 
> 
> I also tried to build without this patch, but it won't compile. The
> following patch seems to depend on this one
> 
> 1257875 New          Makfile: add more warning flags to ensure we catch
> more issues
> 
> error message as below:
> 
> opal/reserv_mem.c: In function ‘make_message’:
> opal/reserv_mem.c:101:2: error: function ‘make_message’ might be a
> candidate for ‘gnu_printf’ format attribute
> [-Werror=suggest-attribute=format]
>   vsnprintf(p, size, fmt, ap);


OK, I'll send a V2 of the Makefile patch. :-(

>   ^~~~~~~~~
>  
> 
> 
>     Colin
> 
>     >
>     > On Wed, Mar 18, 2020 at 6:50 PM Alex Hung <alex.hung at canonical.com
>     <mailto:alex.hung at canonical.com>
>     > <mailto:alex.hung at canonical.com <mailto:alex.hung at canonical.com>>>
>     wrote:
>     >
>     >     On 2020-03-18 5:59 p.m., Colin King wrote:
>     >     > From: Colin Ian King <colin.king at canonical.com
>     <mailto:colin.king at canonical.com>
>     >     <mailto:colin.king at canonical.com
>     <mailto:colin.king at canonical.com>>>
>     >     >
>     >     > Use the printf format attribute specifier to add a little more
>     >     > type checking.  Wrap this with a FWTS_FORMAT macro so that we
>     >     > can no-op this for older compilers that don't support this
>     >     > feature.
>     >     >
>     >     > Signed-off-by: Colin Ian King <colin.king at canonical.com
>     <mailto:colin.king at canonical.com>
>     >     <mailto:colin.king at canonical.com
>     <mailto:colin.king at canonical.com>>>
>     >     > ---
>     >     >  src/lib/include/fwts.h | 8 ++++++++
>     >     >  src/opal/reserv_mem.c  | 2 ++
>     >     >  2 files changed, 10 insertions(+)
>     >     >
>     >     > diff --git a/src/lib/include/fwts.h b/src/lib/include/fwts.h
>     >     > index 7a05befc..7effe818 100644
>     >     > --- a/src/lib/include/fwts.h
>     >     > +++ b/src/lib/include/fwts.h
>     >     > @@ -142,6 +142,14 @@
>     >     >  #define OPTIMIZE0
>     >     >  #endif
>     >     > 
>     >     > +/* print format attribute */
>     >     > +#if ((defined(__GNUC__) && NEED_GNUC(3,2,0)) || \
>     >     > +     (defined(__clang__) && NEED_CLANG(3, 7, 0)))
>     >     > +#define FWTS_FORMAT(func, a, b) __attribute__((format(func,
>     a, b)))
>     >     > +#else
>     >     > +#define FWTS_FORMAT(func, a, b)
>     >     > +#endif
>     >     > +
>     >     >  #define FWTS_UNUSED(var)     (void)var
>     >     > 
>     >     >  #define FWTS_JSON_DATA_PATH  DATAROOTDIR "/fwts"
>     >     > diff --git a/src/opal/reserv_mem.c b/src/opal/reserv_mem.c
>     >     > index f243fb2e..82293c91 100644
>     >     > --- a/src/opal/reserv_mem.c
>     >     > +++ b/src/opal/reserv_mem.c
>     >     > @@ -87,6 +87,8 @@ static int get_config(fwts_framework *fw,
>     >     >       return FWTS_OK;
>     >     >  }
>     >     > 
>     >     > +static char *make_message(const char *fmt, ...)
>     >     FWTS_FORMAT(printf, 1, 2);
>     >     > +
>     >     >  static char *make_message(const char *fmt, ...)
>     >     >  {
>     >     >       char *p;
>     >     >
>     >
>     >     Acked-by: Alex Hung <alex.hung at canonical.com
>     <mailto:alex.hung at canonical.com>
>     >     <mailto:alex.hung at canonical.com <mailto:alex.hung at canonical.com>>>
>     >
>     >
>     >
>     > --
>     > Cheers,
>     > Alex Hung
>     >
> 
> 
> 
> -- 
> Cheers,
> Alex Hung