Message ID | 20220203081820.29521-6-rpalethorpe@suse.com |
---|---|
State | Superseded |
Headers | show |
Series | None | expand |
Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
On Thu, Feb 3, 2022 at 4:20 PM Richard Palethorpe via ltp < ltp@lists.linux.it> wrote: > Allows the name to be formatted which is trivial because we already > copy it into a buffer. Also this removes the init function which is > now just unnecessary verbiage. > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > Suggested-by: Cyril Hrubis <chrubis@suse.cz> > --- > include/tst_cgroup.h | 5 +++-- > lib/tst_cgroup.c | 29 ++++++++++++----------------- > 2 files changed, 15 insertions(+), 19 deletions(-) > > diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h > index 17adefd2b..d7a3433fa 100644 > --- a/include/tst_cgroup.h > +++ b/include/tst_cgroup.h > @@ -126,8 +126,9 @@ void tst_cgroup_init(void); > /* Create a descendant CGroup */ > struct tst_cgroup_group * > tst_cgroup_group_mk(const struct tst_cgroup_group *const parent, > - const char *const group_name) > - __attribute__ ((nonnull, warn_unused_result)); > + const char *const group_name_fmt, ...) > + __attribute__ ((nonnull, warn_unused_result, format (printf, > 2, 3))); > Seems this is too strict for some compiling. e.g. cfs_bandwidth01.c: In function ‘mk_cpu_cgroup’: cfs_bandwidth01.c:64:9: error: format not a string literal and no format arguments [-Werror=format-security] 64 | *cg = tst_cgroup_group_mk(cg_parent, cg_child_name); | ^ cc1: some warnings being treated as errors make: *** [../../../../include/mk/rules.mk:37: cfs_bandwidth01] Error 1 gcc version 11.2.1 20211203 (Red Hat 11.2.1-7) (GCC) > + > const char * > tst_cgroup_group_name(const struct tst_cgroup_group *const cg) > __attribute__ ((nonnull, warn_unused_result)); > diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c > index d9cd6aa8e..66f17575e 100644 > --- a/lib/tst_cgroup.c > +++ b/lib/tst_cgroup.c > @@ -840,21 +840,6 @@ clear_data: > memset(roots, 0, sizeof(roots)); > } > > -__attribute__((nonnull(1))) > -static void cgroup_group_init(struct tst_cgroup_group *const cg, > - const char *const group_name) > -{ > - memset(cg, 0, sizeof(*cg)); > - > - if (!group_name) > - return; > - > - if (strlen(group_name) > NAME_MAX) > - tst_brk(TBROK, "Group name is too long"); > - > - strcpy(cg->group_name, group_name); > -} > - > __attribute__((nonnull(2, 3))) > static void cgroup_group_add_dir(const struct tst_cgroup_group *const > parent, > struct tst_cgroup_group *const cg, > @@ -886,14 +871,24 @@ static void cgroup_group_add_dir(const struct > tst_cgroup_group *const parent, > > struct tst_cgroup_group * > tst_cgroup_group_mk(const struct tst_cgroup_group *const parent, > - const char *const group_name) > + const char *const group_name_fmt, ...) > { > struct tst_cgroup_group *cg; > struct cgroup_dir *const *dir; > struct cgroup_dir *new_dir; > + va_list ap; > + size_t name_len; > > cg = SAFE_MALLOC(sizeof(*cg)); > - cgroup_group_init(cg, group_name); > + memset(cg, 0, sizeof(*cg)); > + > + va_start(ap, group_name_fmt); > + name_len = vsnprintf(cg->group_name, NAME_MAX, > + group_name_fmt, ap); > + va_end(ap); > + > + if (name_len >= NAME_MAX) > + tst_brk(TBROK, "CGroup name is too long"); > > for_each_dir(parent, 0, dir) { > new_dir = SAFE_MALLOC(sizeof(*new_dir)); > -- > 2.34.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp > >
Hi! > Seems this is too strict for some compiling. e.g. > > cfs_bandwidth01.c: In function ???mk_cpu_cgroup???: > cfs_bandwidth01.c:64:9: error: format not a string literal and no format > arguments [-Werror=format-security] > 64 | *cg = tst_cgroup_group_mk(cg_parent, cg_child_name); > | ^ > cc1: some warnings being treated as errors > make: *** [../../../../include/mk/rules.mk:37: cfs_bandwidth01] Error 1 > > gcc version 11.2.1 20211203 (Red Hat 11.2.1-7) (GCC) Ah, right, that's the __attribute__ format printf. I guess that we would have to live with changing all the calls to tst_cgroup_group_mk(foo, "%s", child_name)
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> Seems this is too strict for some compiling. e.g. >> >> cfs_bandwidth01.c: In function ???mk_cpu_cgroup???: >> cfs_bandwidth01.c:64:9: error: format not a string literal and no format >> arguments [-Werror=format-security] >> 64 | *cg = tst_cgroup_group_mk(cg_parent, cg_child_name); >> | ^ >> cc1: some warnings being treated as errors >> make: *** [../../../../include/mk/rules.mk:37: cfs_bandwidth01] Error 1 >> >> gcc version 11.2.1 20211203 (Red Hat 11.2.1-7) (GCC) Sorry I should have done a full rebuild. > > Ah, right, that's the __attribute__ format printf. I guess that we would > have to live with changing all the calls to > tst_cgroup_group_mk(foo, "%s", child_name) Actually it's just calls where the compiler can't tell if child_name is a string literal. Looking at vsnprintf in stdio.h it seems like we should be able to add `__attribute__((format(printf,3,0)))` to mk_cpu_cgroup and then cg_child_name should be guaranteed to be a string literal. Alas it doesn't work, niether does the format_arg attribute in this case. I guess if I made a va arg version of tst_cgroup_group_mk then it would work, but this is starting to get silly. So I'll just do "%s".
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h index 17adefd2b..d7a3433fa 100644 --- a/include/tst_cgroup.h +++ b/include/tst_cgroup.h @@ -126,8 +126,9 @@ void tst_cgroup_init(void); /* Create a descendant CGroup */ struct tst_cgroup_group * tst_cgroup_group_mk(const struct tst_cgroup_group *const parent, - const char *const group_name) - __attribute__ ((nonnull, warn_unused_result)); + const char *const group_name_fmt, ...) + __attribute__ ((nonnull, warn_unused_result, format (printf, 2, 3))); + const char * tst_cgroup_group_name(const struct tst_cgroup_group *const cg) __attribute__ ((nonnull, warn_unused_result)); diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c index d9cd6aa8e..66f17575e 100644 --- a/lib/tst_cgroup.c +++ b/lib/tst_cgroup.c @@ -840,21 +840,6 @@ clear_data: memset(roots, 0, sizeof(roots)); } -__attribute__((nonnull(1))) -static void cgroup_group_init(struct tst_cgroup_group *const cg, - const char *const group_name) -{ - memset(cg, 0, sizeof(*cg)); - - if (!group_name) - return; - - if (strlen(group_name) > NAME_MAX) - tst_brk(TBROK, "Group name is too long"); - - strcpy(cg->group_name, group_name); -} - __attribute__((nonnull(2, 3))) static void cgroup_group_add_dir(const struct tst_cgroup_group *const parent, struct tst_cgroup_group *const cg, @@ -886,14 +871,24 @@ static void cgroup_group_add_dir(const struct tst_cgroup_group *const parent, struct tst_cgroup_group * tst_cgroup_group_mk(const struct tst_cgroup_group *const parent, - const char *const group_name) + const char *const group_name_fmt, ...) { struct tst_cgroup_group *cg; struct cgroup_dir *const *dir; struct cgroup_dir *new_dir; + va_list ap; + size_t name_len; cg = SAFE_MALLOC(sizeof(*cg)); - cgroup_group_init(cg, group_name); + memset(cg, 0, sizeof(*cg)); + + va_start(ap, group_name_fmt); + name_len = vsnprintf(cg->group_name, NAME_MAX, + group_name_fmt, ap); + va_end(ap); + + if (name_len >= NAME_MAX) + tst_brk(TBROK, "CGroup name is too long"); for_each_dir(parent, 0, dir) { new_dir = SAFE_MALLOC(sizeof(*new_dir));
Allows the name to be formatted which is trivial because we already copy it into a buffer. Also this removes the init function which is now just unnecessary verbiage. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> Suggested-by: Cyril Hrubis <chrubis@suse.cz> --- include/tst_cgroup.h | 5 +++-- lib/tst_cgroup.c | 29 ++++++++++++----------------- 2 files changed, 15 insertions(+), 19 deletions(-)