Re: [PATCH 8/8] trace: remove limitation on directory

[Jerin Jacob <jerinjacobk@gmail.com>]

On Wed, Sep 21, 2022 at 5:35 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> Remove arbitrary limit on 12 characters of the file prefix used for the
> directory where to store the traces.
> Simplify the code by relying on dynamic allocations.

Nice one.

>
> Signed-off-by: David Marchand <david.marchand@redhat.com>


Acked-by: Jerin Jacob <jerinj@marvell.com>

> ---
>  lib/eal/common/eal_common_trace_utils.c | 68 +++++++++----------------
>  lib/eal/common/eal_trace.h              |  5 +-
>  2 files changed, 25 insertions(+), 48 deletions(-)
>
> diff --git a/lib/eal/common/eal_common_trace_utils.c b/lib/eal/common/eal_common_trace_utils.c
> index 9b5a41ca12..9e0fe962de 100644
> --- a/lib/eal/common/eal_common_trace_utils.c
> +++ b/lib/eal/common/eal_common_trace_utils.c
> @@ -87,11 +87,11 @@ trace_uuid_generate(void)
>  }
>
>  static int
> -trace_session_name_generate(char *trace_dir)
> +trace_session_name_generate(char **trace_dir)
>  {
> +       char date[sizeof("YYYY-mm-dd-AM-HH-MM-SS")];
>         struct tm *tm_result;
>         time_t tm;
> -       int rc;
>
>         tm = time(NULL);
>         if ((int)tm == -1)
> @@ -101,38 +101,32 @@ trace_session_name_generate(char *trace_dir)
>         if (tm_result == NULL)
>                 goto fail;
>
> -       rc = rte_strscpy(trace_dir, eal_get_hugefile_prefix(),
> -                       TRACE_PREFIX_LEN);
> -       if (rc == -E2BIG)
> -               rc = TRACE_PREFIX_LEN - 1;
> -       trace_dir[rc++] = '-';
> -
> -       rc = strftime(trace_dir + rc, TRACE_DIR_STR_LEN - rc,
> -                       "%Y-%m-%d-%p-%I-%M-%S", tm_result);
> -       if (rc == 0) {
> +       if (strftime(date, sizeof(date), "%Y-%m-%d-%p-%I-%M-%S", tm_result) == 0) {
>                 errno = ENOSPC;
>                 goto fail;
>         }
>
> -       return rc;
> +       if (asprintf(trace_dir, "%s-%s", eal_get_hugefile_prefix(), date) == -1)
> +               goto fail;
> +
> +       return 0;
>  fail:
>         rte_errno = errno;
> -       return -rte_errno;
> +       return -1;
>  }
>
>  static int
>  trace_dir_update(const char *str)
>  {
>         struct trace *trace = trace_obj_get();
> -       int rc, remaining;
> -
> -       remaining = sizeof(trace->dir) - trace->dir_offset;
> -       rc = rte_strscpy(&trace->dir[0] + trace->dir_offset, str, remaining);
> -       if (rc < 0)
> -               goto fail;
> +       char *dir;
> +       int rc;
>
> -       trace->dir_offset += rc;
> -fail:
> +       rc = asprintf(&dir, "%s%s", trace->dir != NULL ? trace->dir : "", str);
> +       if (rc != -1) {
> +               free(trace->dir);
> +               trace->dir = dir;
> +       }
>         return rc;
>  }
>
> @@ -246,22 +240,15 @@ eal_trace_mode_args_save(const char *val)
>  int
>  eal_trace_dir_args_save(char const *val)
>  {
> -       struct trace *trace = trace_obj_get();
>         char *dir_path;
>         int rc;
>
> -       if (strlen(val) >= sizeof(trace->dir) - 1) {
> -               trace_err("input string is too big");
> -               return -ENAMETOOLONG;
> -       }
> -
>         if (asprintf(&dir_path, "%s/", val) == -1) {
>                 trace_err("failed to copy directory: %s", strerror(errno));
>                 return -ENOMEM;
>         }
>
>         rc = trace_dir_update(dir_path);
> -
>         free(dir_path);
>         return rc;
>  }
> @@ -289,10 +276,8 @@ trace_epoch_time_save(void)
>  }
>
>  static int
> -trace_dir_default_path_get(char *dir_path)
> +trace_dir_default_path_get(char **dir_path)
>  {
> -       struct trace *trace = trace_obj_get();
> -       uint32_t size = sizeof(trace->dir);
>         struct passwd *pwd;
>         char *home_dir;
>
> @@ -308,8 +293,8 @@ trace_dir_default_path_get(char *dir_path)
>         }
>
>         /* Append dpdk-traces to directory */
> -       if (snprintf(dir_path, size, "%s/dpdk-traces/", home_dir) < 0)
> -               return -ENAMETOOLONG;
> +       if (asprintf(dir_path, "%s/dpdk-traces/", home_dir) == -1)
> +               return -ENOMEM;
>
>         return 0;
>  }
> @@ -318,25 +303,19 @@ static int
>  trace_mkdir(void)
>  {
>         struct trace *trace = trace_obj_get();
> -       char session[TRACE_DIR_STR_LEN];
>         static bool already_done;
> -       char *dir_path;
> +       char *session;
>         int rc;
>
>         if (already_done)
>                 return 0;
>
> -       if (!trace->dir_offset) {
> -               dir_path = calloc(1, sizeof(trace->dir));
> -               if (dir_path == NULL) {
> -                       trace_err("fail to allocate memory");
> -                       return -ENOMEM;
> -               }
> +       if (trace->dir == NULL) {
> +               char *dir_path;
>
> -               rc = trace_dir_default_path_get(dir_path);
> +               rc = trace_dir_default_path_get(&dir_path);
>                 if (rc < 0) {
>                         trace_err("fail to get default path");
> -                       free(dir_path);
>                         return rc;
>                 }
>
> @@ -354,10 +333,11 @@ trace_mkdir(void)
>                 return -rte_errno;
>         }
>
> -       rc = trace_session_name_generate(session);
> +       rc = trace_session_name_generate(&session);
>         if (rc < 0)
>                 return rc;
>         rc = trace_dir_update(session);
> +       free(session);
>         if (rc < 0)
>                 return rc;
>
> diff --git a/lib/eal/common/eal_trace.h b/lib/eal/common/eal_trace.h
> index 26a18a2c48..d66bcfe198 100644
> --- a/lib/eal/common/eal_trace.h
> +++ b/lib/eal/common/eal_trace.h
> @@ -22,8 +22,6 @@
>  #define trace_crit(fmt, args...) \
>         RTE_LOG(CRIT, EAL, "%s():%u " fmt "\n", __func__, __LINE__, ## args)
>
> -#define TRACE_PREFIX_LEN 12
> -#define TRACE_DIR_STR_LEN (sizeof("YYYY-mm-dd-AM-HH-MM-SS") + TRACE_PREFIX_LEN)
>  #define TRACE_CTF_MAGIC 0xC1FC1FC1
>  #define TRACE_MAX_ARGS 32
>
> @@ -50,8 +48,7 @@ struct trace_arg {
>  };
>
>  struct trace {
> -       char dir[PATH_MAX];
> -       int dir_offset;
> +       char *dir;
>         int register_errno;
>         uint32_t status;
>         enum rte_trace_mode mode;
> --
> 2.37.3
>