[8/8] trace: remove limitation on directory

Message ID 20220921120359.2201131-9-david.marchand@redhat.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers
Series Trace subsystem fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-x86_64-unit-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

David Marchand Sept. 21, 2022, 12:03 p.m. UTC
  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.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/common/eal_common_trace_utils.c | 68 +++++++++----------------
 lib/eal/common/eal_trace.h              |  5 +-
 2 files changed, 25 insertions(+), 48 deletions(-)
  

Comments

Jerin Jacob Oct. 11, 2022, 3:05 p.m. UTC | #1
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
>
  

Patch

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;