| Message ID | 20240325115034.643892-2-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Remove double or trailing slashes in TMPDIR in C API | expand |
Hi, I don't like the approach of directly modifying the process environment. Also, if TMPDIR variable is not set, the newly added strdup() will lead to a memory leak because the function is called multiple times. I recommend adding a static pointer that'll hold the cleaned up path and the string cleanup will run only once on the first call. Then the pointer will be free()d during library cleanup. On 25. 03. 24 12:50, Petr Vorel wrote: > Follow the changes to shell API 273c49793 ("tst_test.sh: Remove possible > double/trailing slashes from TMPDIR") and remove: 1) trailing slash > 2) double slashes. > > This is needed, because some tests compare file path of files which are > in TMPDIR with strcmp() or and extra slashes break it (e.g. chdir01A, > ioctl_loop0[12], mount0[67]). > > Co-developed-by: Cyril Hrubis <chrubis@suse.cz> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > lib/tst_tmpdir.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c > index b73b5c66f..b6e51ba0a 100644 > --- a/lib/tst_tmpdir.c > +++ b/lib/tst_tmpdir.c > @@ -124,16 +124,32 @@ char *tst_get_tmpdir(void) > > const char *tst_get_tmpdir_root(void) > { > - const char *env_tmpdir = getenv("TMPDIR"); > + char *env_tmpdir = getenv("TMPDIR"); > + char prev_c = 0; > + size_t k = 0; > > if (!env_tmpdir) > - env_tmpdir = TEMPDIR; > + env_tmpdir = strdup(TEMPDIR); > > if (env_tmpdir[0] != '/') { > tst_brkm(TBROK, NULL, "You must specify an absolute " > "pathname for environment variable TMPDIR"); > return NULL; > } > + > + for (int i = 0; env_tmpdir[i] != '\0'; i++) { > + if (i) > + prev_c = env_tmpdir[i-1]; > + > + if (env_tmpdir[i] != '/' || prev_c != '/') > + env_tmpdir[k++] = env_tmpdir[i]; > + } > + > + env_tmpdir[k] = '\0'; > + > + if (env_tmpdir[k-1] == '/') > + env_tmpdir[k-1] = '\0'; > + > return env_tmpdir; > } >
Hi Martin, > Hi, > I don't like the approach of directly modifying the process environment. > Also, if TMPDIR variable is not set, the newly added strdup() will lead to a > memory leak because the function is called multiple times. > I recommend adding a static pointer that'll hold the cleaned up path and the > string cleanup will run only once on the first call. Then the pointer will > be free()d during library cleanup. Good, point, thanks! Kind regards, Petr
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c index b73b5c66f..b6e51ba0a 100644 --- a/lib/tst_tmpdir.c +++ b/lib/tst_tmpdir.c @@ -124,16 +124,32 @@ char *tst_get_tmpdir(void) const char *tst_get_tmpdir_root(void) { - const char *env_tmpdir = getenv("TMPDIR"); + char *env_tmpdir = getenv("TMPDIR"); + char prev_c = 0; + size_t k = 0; if (!env_tmpdir) - env_tmpdir = TEMPDIR; + env_tmpdir = strdup(TEMPDIR); if (env_tmpdir[0] != '/') { tst_brkm(TBROK, NULL, "You must specify an absolute " "pathname for environment variable TMPDIR"); return NULL; } + + for (int i = 0; env_tmpdir[i] != '\0'; i++) { + if (i) + prev_c = env_tmpdir[i-1]; + + if (env_tmpdir[i] != '/' || prev_c != '/') + env_tmpdir[k++] = env_tmpdir[i]; + } + + env_tmpdir[k] = '\0'; + + if (env_tmpdir[k-1] == '/') + env_tmpdir[k-1] = '\0'; + return env_tmpdir; }
Follow the changes to shell API 273c49793 ("tst_test.sh: Remove possible double/trailing slashes from TMPDIR") and remove: 1) trailing slash 2) double slashes. This is needed, because some tests compare file path of files which are in TMPDIR with strcmp() or and extra slashes break it (e.g. chdir01A, ioctl_loop0[12], mount0[67]). Co-developed-by: Cyril Hrubis <chrubis@suse.cz> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- lib/tst_tmpdir.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)