| Message ID | 20200318235933.361204-1-colin.king@canonical.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Add GNU printf format attribute specifier for make_message() | expand |
On 2020-03-18 5:59 p.m., Colin King wrote: > From: Colin Ian King <colin.king@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@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@canonical.com>
I found this breaks on 32bit (both PC & ARM) systems... On Wed, Mar 18, 2020 at 6:50 PM Alex Hung <alex.hung@canonical.com> wrote: > On 2020-03-18 5:59 p.m., Colin King wrote: > > From: Colin Ian King <colin.king@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@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@canonical.com> >
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. Colin > > On Wed, Mar 18, 2020 at 6:50 PM Alex Hung <alex.hung@canonical.com > <mailto:alex.hung@canonical.com>> wrote: > > On 2020-03-18 5:59 p.m., Colin King wrote: > > From: Colin Ian King <colin.king@canonical.com > <mailto:colin.king@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@canonical.com > <mailto:colin.king@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@canonical.com > <mailto:alex.hung@canonical.com>> > > > > -- > Cheers, > Alex Hung >
On Thu, Mar 19, 2020 at 6:15 PM Colin Ian King <colin.king@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); ^~~~~~~~~ > > Colin > > > > > On Wed, Mar 18, 2020 at 6:50 PM Alex Hung <alex.hung@canonical.com > > <mailto:alex.hung@canonical.com>> wrote: > > > > On 2020-03-18 5:59 p.m., Colin King wrote: > > > From: Colin Ian King <colin.king@canonical.com > > <mailto:colin.king@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@canonical.com > > <mailto:colin.king@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@canonical.com > > <mailto:alex.hung@canonical.com>> > > > > > > > > -- > > Cheers, > > Alex Hung > > > >
On 20/03/2020 01:47, Alex Hung wrote: > > > On Thu, Mar 19, 2020 at 6:15 PM Colin Ian King <colin.king@canonical.com > <mailto:colin.king@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@canonical.com > <mailto:alex.hung@canonical.com> > > <mailto:alex.hung@canonical.com <mailto:alex.hung@canonical.com>>> > wrote: > > > > On 2020-03-18 5:59 p.m., Colin King wrote: > > > From: Colin Ian King <colin.king@canonical.com > <mailto:colin.king@canonical.com> > > <mailto:colin.king@canonical.com > <mailto:colin.king@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@canonical.com > <mailto:colin.king@canonical.com> > > <mailto:colin.king@canonical.com > <mailto:colin.king@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@canonical.com > <mailto:alex.hung@canonical.com> > > <mailto:alex.hung@canonical.com <mailto:alex.hung@canonical.com>>> > > > > > > > > -- > > Cheers, > > Alex Hung > > > > > > -- > Cheers, > Alex Hung
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;