[v2,4/5] API/cgroup: Make tst_cgroup_group_mk sprintf like

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(-)

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".