* [PATCH for-4.15 0/2] xenstore: Check format printf
@ 2021-03-05 12:40 Julien Grall
2021-03-05 12:40 ` [PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h Julien Grall
2021-03-05 12:40 ` [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() Julien Grall
0 siblings, 2 replies; 16+ messages in thread
From: Julien Grall @ 2021-03-05 12:40 UTC (permalink / raw)
To: xen-devel; +Cc: raphning, iwj, Julien Grall, Wei Liu, Juergen Gross
From: Julien Grall <jgrall@amazon.com>
This patch is a follow-up to Norbert's series [1].
The first patch will define PRINTF_ATTRIBUTE the same way everywhere.
The second patch will check the format printf on a few more calls.
Both patches are candidate for 4.15. They only affects the build system
and therefore would consider them low-risk.
Cheers,
[1] https://lore.kernel.org/xen-devel/20210226144144.9252-1-nmanthey@amazon.de/
Julien Grall (2):
tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h
tools/xenstore: Check the format printf for xprintf() and
barf{,_perror}()
tools/xenstore/talloc.h | 15 ++-------------
tools/xenstore/tdb.h | 6 ++----
tools/xenstore/utils.h | 10 +++++++---
3 files changed, 11 insertions(+), 20 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h 2021-03-05 12:40 [PATCH for-4.15 0/2] xenstore: Check format printf Julien Grall @ 2021-03-05 12:40 ` Julien Grall 2021-03-05 13:00 ` Jürgen Groß 2021-03-05 13:24 ` Ian Jackson 2021-03-05 12:40 ` [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() Julien Grall 1 sibling, 2 replies; 16+ messages in thread From: Julien Grall @ 2021-03-05 12:40 UTC (permalink / raw) To: xen-devel; +Cc: raphning, iwj, Julien Grall, Wei Liu, Juergen Gross From: Julien Grall <jgrall@amazon.com> At the moment PRINTF_ATTRIBUTE() is defined in two places: - tdb.h: Defined as a NOP - talloc.h: Defined as a NOP for GCC older than 3.0 otherwise will add the attribute to check the printf format Xen requires to build with minimum GCC 4.1 and we want to check the printf format for all the printf-like functions. Only implement PRINTF_ATTRIBUTE() once in utils.h and drop the conditional check for GCC < 3.0. Signed-off-by: Julien Grall <jgrall@amazon.com> --- tools/xenstore/talloc.h | 15 ++------------- tools/xenstore/tdb.h | 6 ++---- tools/xenstore/utils.h | 2 ++ 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/tools/xenstore/talloc.h b/tools/xenstore/talloc.h index 71a36e7be06b..a0f4bff25788 100644 --- a/tools/xenstore/talloc.h +++ b/tools/xenstore/talloc.h @@ -26,6 +26,8 @@ #include <sys/types.h> +#include "utils.h" + /* this is only needed for compatibility with the old talloc */ typedef void TALLOC_CTX; @@ -84,19 +86,6 @@ typedef void TALLOC_CTX; #define talloc_destroy(ctx) talloc_free(ctx) #endif -#ifndef PRINTF_ATTRIBUTE -#if (__GNUC__ >= 3) -/** Use gcc attribute to check printf fns. a1 is the 1-based index of - * the parameter containing the format, and a2 the index of the first - * argument. Note that some gcc 2.x versions don't handle this - * properly **/ -#define PRINTF_ATTRIBUTE(a1, a2) __attribute__ ((format (__printf__, a1, a2))) -#else -#define PRINTF_ATTRIBUTE(a1, a2) -#endif -#endif - - /* The following definitions come from talloc.c */ void *_talloc(const void *context, size_t size); void talloc_set_destructor(const void *ptr, int (*destructor)(void *)); diff --git a/tools/xenstore/tdb.h b/tools/xenstore/tdb.h index 557cf727b869..ce3c7339f884 100644 --- a/tools/xenstore/tdb.h +++ b/tools/xenstore/tdb.h @@ -1,6 +1,8 @@ #ifndef __TDB_H__ #define __TDB_H__ +#include "utils.h" + /* Unix SMB/CIFS implementation. @@ -84,10 +86,6 @@ struct tdb_traverse_lock { uint32_t hash; }; -#ifndef PRINTF_ATTRIBUTE -#define PRINTF_ATTRIBUTE(a,b) -#endif - /* this is the context structure that is returned from a db open */ typedef struct tdb_context { char *name; /* the name of the database */ diff --git a/tools/xenstore/utils.h b/tools/xenstore/utils.h index df1cb9a3bac6..3dfb96b556dd 100644 --- a/tools/xenstore/utils.h +++ b/tools/xenstore/utils.h @@ -27,6 +27,8 @@ static inline bool strends(const char *a, const char *b) */ const char *dump_state_align(FILE *fp); +#define PRINTF_ATTRIBUTE(a1, a2) __attribute__((format (printf, a1, a2))) + void barf(const char *fmt, ...) __attribute__((noreturn)); void barf_perror(const char *fmt, ...) __attribute__((noreturn)); -- 2.17.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h 2021-03-05 12:40 ` [PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h Julien Grall @ 2021-03-05 13:00 ` Jürgen Groß 2021-03-05 13:24 ` Ian Jackson 1 sibling, 0 replies; 16+ messages in thread From: Jürgen Groß @ 2021-03-05 13:00 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: raphning, iwj, Julien Grall, Wei Liu [-- Attachment #1.1.1: Type: text/plain, Size: 659 bytes --] On 05.03.21 13:40, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > At the moment PRINTF_ATTRIBUTE() is defined in two places: > - tdb.h: Defined as a NOP > - talloc.h: Defined as a NOP for GCC older than 3.0 otherwise will > add the attribute to check the printf format > > Xen requires to build with minimum GCC 4.1 and we want to check the > printf format for all the printf-like functions. > > Only implement PRINTF_ATTRIBUTE() once in utils.h and drop the > conditional check for GCC < 3.0. > > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen [-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h 2021-03-05 12:40 ` [PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h Julien Grall 2021-03-05 13:00 ` Jürgen Groß @ 2021-03-05 13:24 ` Ian Jackson 1 sibling, 0 replies; 16+ messages in thread From: Ian Jackson @ 2021-03-05 13:24 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, raphning, Julien Grall, Wei Liu, Juergen Gross Julien Grall writes ("[PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h"): > From: Julien Grall <jgrall@amazon.com> > > At the moment PRINTF_ATTRIBUTE() is defined in two places: > - tdb.h: Defined as a NOP > - talloc.h: Defined as a NOP for GCC older than 3.0 otherwise will > add the attribute to check the printf format > > Xen requires to build with minimum GCC 4.1 and we want to check the > printf format for all the printf-like functions. > > Only implement PRINTF_ATTRIBUTE() once in utils.h and drop the > conditional check for GCC < 3.0. Release-Acked-by: Ian Jackson <iwj@xenproject.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 12:40 [PATCH for-4.15 0/2] xenstore: Check format printf Julien Grall 2021-03-05 12:40 ` [PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h Julien Grall @ 2021-03-05 12:40 ` Julien Grall 2021-03-05 13:01 ` Jürgen Groß 2021-03-05 13:24 ` Ian Jackson 1 sibling, 2 replies; 16+ messages in thread From: Julien Grall @ 2021-03-05 12:40 UTC (permalink / raw) To: xen-devel; +Cc: raphning, iwj, Julien Grall, Wei Liu, Juergen Gross From: Julien Grall <jgrall@amazon.com> Allow GCC to analyze the format printf for xprintf() and barf{,_perror}(). Take the opportunity to define __noreturn to make the prototype for barf{,_perror})() easier to read. Signed-off-by: Julien Grall <jgrall@amazon.com> --- tools/xenstore/utils.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/xenstore/utils.h b/tools/xenstore/utils.h index 3dfb96b556dd..ccfb9b8fb699 100644 --- a/tools/xenstore/utils.h +++ b/tools/xenstore/utils.h @@ -29,10 +29,12 @@ const char *dump_state_align(FILE *fp); #define PRINTF_ATTRIBUTE(a1, a2) __attribute__((format (printf, a1, a2))) -void barf(const char *fmt, ...) __attribute__((noreturn)); -void barf_perror(const char *fmt, ...) __attribute__((noreturn)); +#define __noreturn __attribute__((noreturn)) -extern void (*xprintf)(const char *fmt, ...); +void barf(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); +void barf_perror(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); + +extern void (*xprintf)(const char *fmt, ...) PRINTF_ATTRIBUTE(1, 2); #define eprintf(_fmt, _args...) xprintf("[ERR] %s" _fmt, __FUNCTION__, ##_args) -- 2.17.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 12:40 ` [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() Julien Grall @ 2021-03-05 13:01 ` Jürgen Groß 2021-03-05 13:27 ` Ian Jackson 2021-03-05 13:45 ` Jan Beulich 2021-03-05 13:24 ` Ian Jackson 1 sibling, 2 replies; 16+ messages in thread From: Jürgen Groß @ 2021-03-05 13:01 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: raphning, iwj, Julien Grall, Wei Liu [-- Attachment #1.1.1: Type: text/plain, Size: 1365 bytes --] On 05.03.21 13:40, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Allow GCC to analyze the format printf for xprintf() and > barf{,_perror}(). > > Take the opportunity to define __noreturn to make the prototype for > barf{,_perror})() easier to read. > > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Juergen Gross <jgross@suse.com> But I would prefer, if ... > --- > tools/xenstore/utils.h | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/tools/xenstore/utils.h b/tools/xenstore/utils.h > index 3dfb96b556dd..ccfb9b8fb699 100644 > --- a/tools/xenstore/utils.h > +++ b/tools/xenstore/utils.h > @@ -29,10 +29,12 @@ const char *dump_state_align(FILE *fp); > > #define PRINTF_ATTRIBUTE(a1, a2) __attribute__((format (printf, a1, a2))) > > -void barf(const char *fmt, ...) __attribute__((noreturn)); > -void barf_perror(const char *fmt, ...) __attribute__((noreturn)); > +#define __noreturn __attribute__((noreturn)) > > -extern void (*xprintf)(const char *fmt, ...); > +void barf(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); > +void barf_perror(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); > + > +extern void (*xprintf)(const char *fmt, ...) PRINTF_ATTRIBUTE(1, 2); ... the extern here would be dropped. Juergen [-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 13:01 ` Jürgen Groß @ 2021-03-05 13:27 ` Ian Jackson 2021-03-05 13:32 ` Julien Grall 2021-03-05 13:45 ` Jan Beulich 1 sibling, 1 reply; 16+ messages in thread From: Ian Jackson @ 2021-03-05 13:27 UTC (permalink / raw) To: Jürgen Groß Cc: Julien Grall, xen-devel, raphning, Julien Grall, Wei Liu Jürgen Groß writes ("Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}()"): > On 05.03.21 13:40, Julien Grall wrote: > > -extern void (*xprintf)(const char *fmt, ...); > > +void barf(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); > > +void barf_perror(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); > > + > > +extern void (*xprintf)(const char *fmt, ...) PRINTF_ATTRIBUTE(1, 2); > > ... the extern here would be dropped. With my RM hat on I don't have an opinion on that and my R-A can stand. With my maintainer hat on I agree with Jürgen's style opinion - it's nicer without the "extern", but I'm also happy with the patch as is. Ian. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 13:27 ` Ian Jackson @ 2021-03-05 13:32 ` Julien Grall 0 siblings, 0 replies; 16+ messages in thread From: Julien Grall @ 2021-03-05 13:32 UTC (permalink / raw) To: Ian Jackson, Jürgen Groß Cc: xen-devel, raphning, Julien Grall, Wei Liu Hi Ian, On 05/03/2021 13:27, Ian Jackson wrote: > Jürgen Groß writes ("Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}()"): >> On 05.03.21 13:40, Julien Grall wrote: >>> -extern void (*xprintf)(const char *fmt, ...); >>> +void barf(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); >>> +void barf_perror(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); >>> + >>> +extern void (*xprintf)(const char *fmt, ...) PRINTF_ATTRIBUTE(1, 2); >> >> ... the extern here would be dropped. > > With my RM hat on I don't have an opinion on that and my R-A can > stand. > > With my maintainer hat on I agree with Jürgen's style opinion - it's > nicer without the "extern", but I'm also happy with the patch as is. I agree with Juergen's style opinion. I will do the modification on commit. I will also update the last sentence of the commit to: " Take the opportunity to: * remove extern from the function prototype for consistency * define __noreturn to make the prototype for barf{,_perror})() easier to read. " Cheers, > Ian. > -- Julien Grall ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 13:01 ` Jürgen Groß 2021-03-05 13:27 ` Ian Jackson @ 2021-03-05 13:45 ` Jan Beulich 2021-03-05 13:48 ` Andrew Cooper 2021-03-05 13:48 ` Julien Grall 1 sibling, 2 replies; 16+ messages in thread From: Jan Beulich @ 2021-03-05 13:45 UTC (permalink / raw) To: Jürgen Groß, Julien Grall Cc: raphning, iwj, Julien Grall, Wei Liu, xen-devel On 05.03.2021 14:01, Jürgen Groß wrote: > On 05.03.21 13:40, Julien Grall wrote: >> From: Julien Grall <jgrall@amazon.com> >> --- a/tools/xenstore/utils.h >> +++ b/tools/xenstore/utils.h >> @@ -29,10 +29,12 @@ const char *dump_state_align(FILE *fp); >> >> #define PRINTF_ATTRIBUTE(a1, a2) __attribute__((format (printf, a1, a2))) >> >> -void barf(const char *fmt, ...) __attribute__((noreturn)); >> -void barf_perror(const char *fmt, ...) __attribute__((noreturn)); >> +#define __noreturn __attribute__((noreturn)) >> >> -extern void (*xprintf)(const char *fmt, ...); >> +void barf(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); >> +void barf_perror(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); >> + >> +extern void (*xprintf)(const char *fmt, ...) PRINTF_ATTRIBUTE(1, 2); > > ... the extern here would be dropped. But this isn't a function declaration, but that of a data object. With the extern dropped, a common symbol will appear in every CU. Jan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 13:45 ` Jan Beulich @ 2021-03-05 13:48 ` Andrew Cooper 2021-03-05 13:48 ` Julien Grall 1 sibling, 0 replies; 16+ messages in thread From: Andrew Cooper @ 2021-03-05 13:48 UTC (permalink / raw) To: Jan Beulich, Jürgen Groß, Julien Grall Cc: raphning, iwj, Julien Grall, Wei Liu, xen-devel On 05/03/2021 13:45, Jan Beulich wrote: > On 05.03.2021 14:01, Jürgen Groß wrote: >> On 05.03.21 13:40, Julien Grall wrote: >>> From: Julien Grall <jgrall@amazon.com> >>> --- a/tools/xenstore/utils.h >>> +++ b/tools/xenstore/utils.h >>> @@ -29,10 +29,12 @@ const char *dump_state_align(FILE *fp); >>> >>> #define PRINTF_ATTRIBUTE(a1, a2) __attribute__((format (printf, a1, a2))) >>> >>> -void barf(const char *fmt, ...) __attribute__((noreturn)); >>> -void barf_perror(const char *fmt, ...) __attribute__((noreturn)); >>> +#define __noreturn __attribute__((noreturn)) >>> >>> -extern void (*xprintf)(const char *fmt, ...); >>> +void barf(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); >>> +void barf_perror(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); >>> + >>> +extern void (*xprintf)(const char *fmt, ...) PRINTF_ATTRIBUTE(1, 2); >> ... the extern here would be dropped. > But this isn't a function declaration, but that of a data object. > With the extern dropped, a common symbol will appear in every CU. Correct, and some of the containers in Gitlab CI really ought to notice because GCC 9(? IIRC) defaulted to -fno-common. ~Andrew ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 13:45 ` Jan Beulich 2021-03-05 13:48 ` Andrew Cooper @ 2021-03-05 13:48 ` Julien Grall 2021-03-05 13:58 ` Ian Jackson 1 sibling, 1 reply; 16+ messages in thread From: Julien Grall @ 2021-03-05 13:48 UTC (permalink / raw) To: Jan Beulich, Jürgen Groß Cc: raphning, iwj, Julien Grall, Wei Liu, xen-devel Hi Jan, On 05/03/2021 13:45, Jan Beulich wrote: > On 05.03.2021 14:01, Jürgen Groß wrote: >> On 05.03.21 13:40, Julien Grall wrote: >>> From: Julien Grall <jgrall@amazon.com> >>> --- a/tools/xenstore/utils.h >>> +++ b/tools/xenstore/utils.h >>> @@ -29,10 +29,12 @@ const char *dump_state_align(FILE *fp); >>> >>> #define PRINTF_ATTRIBUTE(a1, a2) __attribute__((format (printf, a1, a2))) >>> >>> -void barf(const char *fmt, ...) __attribute__((noreturn)); >>> -void barf_perror(const char *fmt, ...) __attribute__((noreturn)); >>> +#define __noreturn __attribute__((noreturn)) >>> >>> -extern void (*xprintf)(const char *fmt, ...); >>> +void barf(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); >>> +void barf_perror(const char *fmt, ...) __noreturn PRINTF_ATTRIBUTE(1, 2); >>> + >>> +extern void (*xprintf)(const char *fmt, ...) PRINTF_ATTRIBUTE(1, 2); >> >> ... the extern here would be dropped. > > But this isn't a function declaration, but that of a data object. > With the extern dropped, a common symbol will appear in every CU. Urgh, you are right. Actually, the extern was added recently by Anthony: dacdbf7088d6a3705a9831e73991c2b14c519a65 ("tools/xenstore: mark variable in header as extern") I completely forgot it despite I needed to backport the patch to our downstream Xen. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 13:48 ` Julien Grall @ 2021-03-05 13:58 ` Ian Jackson 2021-03-06 19:37 ` Julien Grall 0 siblings, 1 reply; 16+ messages in thread From: Ian Jackson @ 2021-03-05 13:58 UTC (permalink / raw) To: Julien Grall Cc: Jan Beulich, Jürgen Groß, raphning, Julien Grall, Wei Liu, xen-devel Julien Grall writes ("Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}()"): > Urgh, you are right. Actually, the extern was added recently by Anthony: > > dacdbf7088d6a3705a9831e73991c2b14c519a65 ("tools/xenstore: mark variable > in header as extern") > > I completely forgot it despite I needed to backport the patch to our > downstream Xen. How horrible. Maybe we could add a comment to the code, next to the declaration, about this crazy situation. Ian. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 13:58 ` Ian Jackson @ 2021-03-06 19:37 ` Julien Grall 2021-03-08 9:44 ` Ian Jackson 0 siblings, 1 reply; 16+ messages in thread From: Julien Grall @ 2021-03-06 19:37 UTC (permalink / raw) To: Ian Jackson Cc: Jan Beulich, Jürgen Groß, raphning, Julien Grall, Wei Liu, xen-devel Hi Ian, On 05/03/2021 13:58, Ian Jackson wrote: > Julien Grall writes ("Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}()"): >> Urgh, you are right. Actually, the extern was added recently by Anthony: >> >> dacdbf7088d6a3705a9831e73991c2b14c519a65 ("tools/xenstore: mark variable >> in header as extern") >> >> I completely forgot it despite I needed to backport the patch to our >> downstream Xen. > > How horrible. > > Maybe we could add a comment to the code, next to the declaration, > about this crazy situation. Would the following comment work for you? /* Function pointer as xprintf() can be configured at runtime. */ I can fold it in my patch while committing. Cheers, > > Ian. > -- Julien Grall ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-06 19:37 ` Julien Grall @ 2021-03-08 9:44 ` Ian Jackson 2021-03-11 9:48 ` Julien Grall 0 siblings, 1 reply; 16+ messages in thread From: Ian Jackson @ 2021-03-08 9:44 UTC (permalink / raw) To: Julien Grall Cc: Jan Beulich, Jürgen Groß, raphning, Julien Grall, Wei Liu, xen-devel Julien Grall writes ("Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}()"): > Would the following comment work for you? > > /* Function pointer as xprintf() can be configured at runtime. */ > > I can fold it in my patch while committing. Sure, thanks. FTAOD Reviewed-by: Ian Jackson <iwj@xenproject.org> Release-Acked-by: Ian Jackson <iwj@xenproject.org> to that comment addition. Ian. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-08 9:44 ` Ian Jackson @ 2021-03-11 9:48 ` Julien Grall 0 siblings, 0 replies; 16+ messages in thread From: Julien Grall @ 2021-03-11 9:48 UTC (permalink / raw) To: Ian Jackson Cc: Jan Beulich, Jürgen Groß, raphning, Julien Grall, Wei Liu, xen-devel Hi Ian, On 08/03/2021 09:44, Ian Jackson wrote: > Julien Grall writes ("Re: [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}()"): >> Would the following comment work for you? >> >> /* Function pointer as xprintf() can be configured at runtime. */ >> >> I can fold it in my patch while committing. > > Sure, thanks. FTAOD > > Reviewed-by: Ian Jackson <iwj@xenproject.org> > Release-Acked-by: Ian Jackson <iwj@xenproject.org> > > to that comment addition. Thanks! I have committed the two patches. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() 2021-03-05 12:40 ` [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() Julien Grall 2021-03-05 13:01 ` Jürgen Groß @ 2021-03-05 13:24 ` Ian Jackson 1 sibling, 0 replies; 16+ messages in thread From: Ian Jackson @ 2021-03-05 13:24 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, raphning, Julien Grall, Wei Liu, Juergen Gross Julien Grall writes ("[PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}()"): > From: Julien Grall <jgrall@amazon.com> > > Allow GCC to analyze the format printf for xprintf() and > barf{,_perror}(). > > Take the opportunity to define __noreturn to make the prototype for > barf{,_perror})() easier to read. Release-Acked-by: Ian Jackson <iwj@xenproject.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-03-11 9:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 12:40 [PATCH for-4.15 0/2] xenstore: Check format printf Julien Grall
2021-03-05 12:40 ` [PATCH for-4.15 1/2] tools/xenstore: Consolidate PRINTF_ATTRIBUTE() in utils.h Julien Grall
2021-03-05 13:00 ` Jürgen Groß
2021-03-05 13:24 ` Ian Jackson
2021-03-05 12:40 ` [PATCH for-4.15 2/2] tools/xenstore: Check the format printf for xprintf() and barf{,_perror}() Julien Grall
2021-03-05 13:01 ` Jürgen Groß
2021-03-05 13:27 ` Ian Jackson
2021-03-05 13:32 ` Julien Grall
2021-03-05 13:45 ` Jan Beulich
2021-03-05 13:48 ` Andrew Cooper
2021-03-05 13:48 ` Julien Grall
2021-03-05 13:58 ` Ian Jackson
2021-03-06 19:37 ` Julien Grall
2021-03-08 9:44 ` Ian Jackson
2021-03-11 9:48 ` Julien Grall
2021-03-05 13:24 ` Ian Jackson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).