[LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[Petr Vorel]

Similarly to previous commit, which normalized TMPDIR for shell API,
do the same for C API.

Unlike for shell API, here we modify $TMPDIR directly, because
tst_get_tmpdir_root() is used o more places.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Follow up of the same change in shell API:
https://lore.kernel.org/ltp/20230412073953.1983857-1-pvorel@suse.cz/

Kind regards,
Petr

 lib/tst_tmpdir.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index b73b5c66f..8db5c47e8 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -124,16 +124,28 @@ char *tst_get_tmpdir(void)
 
 const char *tst_get_tmpdir_root(void)
 {
-	const char *env_tmpdir = getenv("TMPDIR");
+	char *env_tmpdir = getenv("TMPDIR");
 
-	if (!env_tmpdir)
+	if (env_tmpdir) {
+		/* remove duplicate slashes */
+		for (char *p = env_tmpdir, *q = env_tmpdir; *q;) {
+			if (*++q != '/' || *p != '/')
+				*++p = *q;
+		}
+		/* Remove slash on the last place  */
+		size_t last = strlen(env_tmpdir)-1;
+		if (env_tmpdir[last] == '/')
+			env_tmpdir[last] = '\0';
+	} else {
 		env_tmpdir = TEMPDIR;
+	}
 
 	if (env_tmpdir[0] != '/') {
 		tst_brkm(TBROK, NULL, "You must specify an absolute "
 				"pathname for environment variable TMPDIR");
 		return NULL;
 	}
+
 	return env_tmpdir;
 }
 
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[Li Wang]

Hi Petr,

On Thu, Apr 13, 2023 at 7:14 PM Petr Vorel <pvorel@suse.cz> wrote:

> Similarly to previous commit, which normalized TMPDIR for shell API,
> do the same for C API.
>
> Unlike for shell API, here we modify $TMPDIR directly, because
> tst_get_tmpdir_root() is used o more places.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Follow up of the same change in shell API:
> https://lore.kernel.org/ltp/20230412073953.1983857-1-pvorel@suse.cz/
>
> Kind regards,
> Petr
>
>  lib/tst_tmpdir.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index b73b5c66f..8db5c47e8 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -124,16 +124,28 @@ char *tst_get_tmpdir(void)
>
>  const char *tst_get_tmpdir_root(void)
>  {
> -       const char *env_tmpdir = getenv("TMPDIR");
> +       char *env_tmpdir = getenv("TMPDIR");
>

It seems that modifying the environment variables is generally
not a good practice.

The getenv() function returns a pointer to the value of an
environment variable, which is stored in the memory managed
by the system. Any attempt to modify this memory directly can
cause unexpected behavior or even crash the program.

Instead of modifying the return value of getenv(), it is recommended
to create a copy of the value and modify the copy instead.

Or, the simplest way I guess is just TBROK and tell users why
this TMPDIR is unusable.



> -       if (!env_tmpdir)
> +       if (env_tmpdir) {
> +               /* remove duplicate slashes */
> +               for (char *p = env_tmpdir, *q = env_tmpdir; *q;) {
> +                       if (*++q != '/' || *p != '/')
> +                               *++p = *q;
> +               }
> +               /* Remove slash on the last place  */
> +               size_t last = strlen(env_tmpdir)-1;
> +               if (env_tmpdir[last] == '/')
> +                       env_tmpdir[last] = '\0';
> +       } else {
>                 env_tmpdir = TEMPDIR;
> +       }
>
>         if (env_tmpdir[0] != '/') {
>                 tst_brkm(TBROK, NULL, "You must specify an absolute "
>                                 "pathname for environment variable
> TMPDIR");
>                 return NULL;
>         }
> +
>         return env_tmpdir;
>  }
>
> --
> 2.40.0
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[Li Wang]

On Wed, Apr 19, 2023 at 2:47 PM Li Wang <liwang@redhat.com> wrote:

> Hi Petr,
>
> On Thu, Apr 13, 2023 at 7:14 PM Petr Vorel <pvorel@suse.cz> wrote:
>
>> Similarly to previous commit, which normalized TMPDIR for shell API,
>> do the same for C API.
>>
>> Unlike for shell API, here we modify $TMPDIR directly, because
>> tst_get_tmpdir_root() is used o more places.
>>
>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>> ---
>> Follow up of the same change in shell API:
>> https://lore.kernel.org/ltp/20230412073953.1983857-1-pvorel@suse.cz/
>>
>> Kind regards,
>> Petr
>>
>>  lib/tst_tmpdir.c | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
>> index b73b5c66f..8db5c47e8 100644
>> --- a/lib/tst_tmpdir.c
>> +++ b/lib/tst_tmpdir.c
>> @@ -124,16 +124,28 @@ char *tst_get_tmpdir(void)
>>
>>  const char *tst_get_tmpdir_root(void)
>>  {
>> -       const char *env_tmpdir = getenv("TMPDIR");
>> +       char *env_tmpdir = getenv("TMPDIR");
>>
>
> It seems that modifying the environment variables is generally
> not a good practice.
>
> The getenv() function returns a pointer to the value of an
> environment variable, which is stored in the memory managed
> by the system. Any attempt to modify this memory directly can
> cause unexpected behavior or even crash the program.
>
> Instead of modifying the return value of getenv(), it is recommended
> to create a copy of the value and modify the copy instead.
>

Btw, the wise method is to use setenv() function to reset
environment variables if really needed.

This is a different part of shell API I have to say.



> Or, the simplest way I guess is just TBROK and tell users why
> this TMPDIR is unusable.
>
>
>
>> -       if (!env_tmpdir)
>> +       if (env_tmpdir) {
>> +               /* remove duplicate slashes */
>> +               for (char *p = env_tmpdir, *q = env_tmpdir; *q;) {
>> +                       if (*++q != '/' || *p != '/')
>> +                               *++p = *q;
>> +               }
>> +               /* Remove slash on the last place  */
>> +               size_t last = strlen(env_tmpdir)-1;
>> +               if (env_tmpdir[last] == '/')
>> +                       env_tmpdir[last] = '\0';
>> +       } else {
>>                 env_tmpdir = TEMPDIR;
>> +       }
>>
>>         if (env_tmpdir[0] != '/') {
>>                 tst_brkm(TBROK, NULL, "You must specify an absolute "
>>                                 "pathname for environment variable
>> TMPDIR");
>>                 return NULL;
>>         }
>> +
>>         return env_tmpdir;
>>  }
>>
>> --
>> 2.40.0
>>
>>
>
> --
> Regards,
> Li Wang
>


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[Petr Vorel]

Hi Li,

> On Wed, Apr 19, 2023 at 2:47 PM Li Wang <liwang@redhat.com> wrote:

> > Hi Petr,

> > On Thu, Apr 13, 2023 at 7:14 PM Petr Vorel <pvorel@suse.cz> wrote:

> >> Similarly to previous commit, which normalized TMPDIR for shell API,
> >> do the same for C API.

> >> Unlike for shell API, here we modify $TMPDIR directly, because
> >> tst_get_tmpdir_root() is used o more places.

> >> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> >> ---
> >> Follow up of the same change in shell API:
> >> https://lore.kernel.org/ltp/20230412073953.1983857-1-pvorel@suse.cz/

> >> Kind regards,
> >> Petr

> >>  lib/tst_tmpdir.c | 16 ++++++++++++++--
> >>  1 file changed, 14 insertions(+), 2 deletions(-)

> >> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> >> index b73b5c66f..8db5c47e8 100644
> >> --- a/lib/tst_tmpdir.c
> >> +++ b/lib/tst_tmpdir.c
> >> @@ -124,16 +124,28 @@ char *tst_get_tmpdir(void)

> >>  const char *tst_get_tmpdir_root(void)
> >>  {
> >> -       const char *env_tmpdir = getenv("TMPDIR");
> >> +       char *env_tmpdir = getenv("TMPDIR");


> > It seems that modifying the environment variables is generally
> > not a good practice.

> > The getenv() function returns a pointer to the value of an
> > environment variable, which is stored in the memory managed
> > by the system. Any attempt to modify this memory directly can
> > cause unexpected behavior or even crash the program.

> > Instead of modifying the return value of getenv(), it is recommended
> > to create a copy of the value and modify the copy instead.

Do you mean to use strdup()?

Also man getenv(3) says:

       As typically implemented, getenv() returns a pointer to a string
       within the environment list.  The caller must take care not to
       modify this string, since that would change the environment of
       the process.

=> I would not mind $TMPDIR got updated in the environment.

> Btw, the wise method is to use setenv() function to reset
> environment variables if really needed.

Well, I don't know any C test which needs it (only NFS tests which are shell
tests). But I wanted to have the same behavior in both APIs.

> This is a different part of shell API I have to say.

Yes, the behavior is slightly different from shell API [1],
where it modifies $TST_TMPDIR (keep $TMPDIR untouched).


> > Or, the simplest way I guess is just TBROK and tell users why
> > this TMPDIR is unusable.

If you prefer it's better to TBROK on:
* double slashes
* trailing slash

I can do that. But at least on trailing slash looks to me quite strict.

Whatever path we choose, I'd need also to update docs. BTW the need
to absolute path for TMPDIR is only in C - shell happily takes relative path
and IMHO it's not documented.

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/project/ltp/patch/20230412073953.1983857-1-pvorel@suse.cz/

> >> -       if (!env_tmpdir)
> >> +       if (env_tmpdir) {
> >> +               /* remove duplicate slashes */
> >> +               for (char *p = env_tmpdir, *q = env_tmpdir; *q;) {
> >> +                       if (*++q != '/' || *p != '/')
> >> +                               *++p = *q;
> >> +               }
> >> +               /* Remove slash on the last place  */
> >> +               size_t last = strlen(env_tmpdir)-1;
> >> +               if (env_tmpdir[last] == '/')
> >> +                       env_tmpdir[last] = '\0';
> >> +       } else {
> >>                 env_tmpdir = TEMPDIR;
> >> +       }

> >>         if (env_tmpdir[0] != '/') {
> >>                 tst_brkm(TBROK, NULL, "You must specify an absolute "
> >>                                 "pathname for environment variable
> >> TMPDIR");
> >>                 return NULL;
> >>         }
> >> +
> >>         return env_tmpdir;
> >>  }

> >> --
> >> 2.40.0



> > --
> > Regards,
> > Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[Li Wang]

On Wed, Apr 19, 2023 at 5:59 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > On Wed, Apr 19, 2023 at 2:47 PM Li Wang <liwang@redhat.com> wrote:
>
> > > Hi Petr,
>
> > > On Thu, Apr 13, 2023 at 7:14 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > >> Similarly to previous commit, which normalized TMPDIR for shell API,
> > >> do the same for C API.
>
> > >> Unlike for shell API, here we modify $TMPDIR directly, because
> > >> tst_get_tmpdir_root() is used o more places.
>
> > >> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > >> ---
> > >> Follow up of the same change in shell API:
> > >> https://lore.kernel.org/ltp/20230412073953.1983857-1-pvorel@suse.cz/
>
> > >> Kind regards,
> > >> Petr
>
> > >>  lib/tst_tmpdir.c | 16 ++++++++++++++--
> > >>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> > >> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> > >> index b73b5c66f..8db5c47e8 100644
> > >> --- a/lib/tst_tmpdir.c
> > >> +++ b/lib/tst_tmpdir.c
> > >> @@ -124,16 +124,28 @@ char *tst_get_tmpdir(void)
>
> > >>  const char *tst_get_tmpdir_root(void)
> > >>  {
> > >> -       const char *env_tmpdir = getenv("TMPDIR");
> > >> +       char *env_tmpdir = getenv("TMPDIR");
>
>
> > > It seems that modifying the environment variables is generally
> > > not a good practice.
>
> > > The getenv() function returns a pointer to the value of an
> > > environment variable, which is stored in the memory managed
> > > by the system. Any attempt to modify this memory directly can
> > > cause unexpected behavior or even crash the program.
>
> > > Instead of modifying the return value of getenv(), it is recommended
> > > to create a copy of the value and modify the copy instead.
>
> Do you mean to use strdup()?
>

Yeah, something like that, or we declare a buffer, and use strcpy()
to copy the string pointed to by the return value of getenv() into the
buffer that we can safely modify.

I prefer it in this way.



>
> Also man getenv(3) says:
>
>        As typically implemented, getenv() returns a pointer to a string
>        within the environment list.  The caller must take care not to
>        modify this string, since that would change the environment of
>        the process.
>
> => I would not mind $TMPDIR got updated in the environment.
>
> > Btw, the wise method is to use setenv() function to reset
> > environment variables if really needed.
>
> Well, I don't know any C test which needs it (only NFS tests which are
> shell
> tests). But I wanted to have the same behavior in both APIs.
>
> > This is a different part of shell API I have to say.
>
> Yes, the behavior is slightly different from shell API [1],
> where it modifies $TST_TMPDIR (keep $TMPDIR untouched).
>
>
> > > Or, the simplest way I guess is just TBROK and tell users why
> > > this TMPDIR is unusable.
>
> If you prefer it's better to TBROK on:
> * double slashes
> * trailing slash
>
> I can do that. But at least on trailing slash looks to me quite strict.
>

-1, trailing and double slash all accepted by shell in command line,
maybe we shouldn't set a more strict policy than that.



>
> Whatever path we choose, I'd need also to update docs. BTW the need
> to absolute path for TMPDIR is only in C - shell happily takes relative
> path
> and IMHO it's not documented.
>
> Kind regards,
> Petr
>
> [1]
> https://patchwork.ozlabs.org/project/ltp/patch/20230412073953.1983857-1-pvorel@suse.cz/
>
> > >> -       if (!env_tmpdir)
> > >> +       if (env_tmpdir) {
> > >> +               /* remove duplicate slashes */
> > >> +               for (char *p = env_tmpdir, *q = env_tmpdir; *q;) {
> > >> +                       if (*++q != '/' || *p != '/')
> > >> +                               *++p = *q;
> > >> +               }
> > >> +               /* Remove slash on the last place  */
> > >> +               size_t last = strlen(env_tmpdir)-1;
> > >> +               if (env_tmpdir[last] == '/')
> > >> +                       env_tmpdir[last] = '\0';
> > >> +       } else {
> > >>                 env_tmpdir = TEMPDIR;
> > >> +       }
>
> > >>         if (env_tmpdir[0] != '/') {
> > >>                 tst_brkm(TBROK, NULL, "You must specify an absolute "
> > >>                                 "pathname for environment variable
> > >> TMPDIR");
> > >>                 return NULL;
> > >>         }
> > >> +
> > >>         return env_tmpdir;
> > >>  }
>
> > >> --
> > >> 2.40.0
>
>
>
> > > --
> > > Regards,
> > > Li Wang
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[Petr Vorel]

Hi Li,
...
> > > >> +++ b/lib/tst_tmpdir.c
> > > >> @@ -124,16 +124,28 @@ char *tst_get_tmpdir(void)

> > > >>  const char *tst_get_tmpdir_root(void)
> > > >>  {
> > > >> -       const char *env_tmpdir = getenv("TMPDIR");
> > > >> +       char *env_tmpdir = getenv("TMPDIR");


> > > > It seems that modifying the environment variables is generally
> > > > not a good practice.

> > > > The getenv() function returns a pointer to the value of an
> > > > environment variable, which is stored in the memory managed
> > > > by the system. Any attempt to modify this memory directly can
> > > > cause unexpected behavior or even crash the program.

> > > > Instead of modifying the return value of getenv(), it is recommended
> > > > to create a copy of the value and modify the copy instead.

> > Do you mean to use strdup()?


> Yeah, something like that, or we declare a buffer, and use strcpy()
> to copy the string pointed to by the return value of getenv() into the
> buffer that we can safely modify.

> I prefer it in this way.


Sure, I'll post new version with this. Until then I keep this patch open if
anybody wants to comment it.

> > Also man getenv(3) says:

> >        As typically implemented, getenv() returns a pointer to a string
> >        within the environment list.  The caller must take care not to
> >        modify this string, since that would change the environment of
> >        the process.

> > => I would not mind $TMPDIR got updated in the environment.

> > > Btw, the wise method is to use setenv() function to reset
> > > environment variables if really needed.

> > Well, I don't know any C test which needs it (only NFS tests which are
> > shell
> > tests). But I wanted to have the same behavior in both APIs.

> > > This is a different part of shell API I have to say.

> > Yes, the behavior is slightly different from shell API [1],
> > where it modifies $TST_TMPDIR (keep $TMPDIR untouched).


> > > > Or, the simplest way I guess is just TBROK and tell users why
> > > > this TMPDIR is unusable.

> > If you prefer it's better to TBROK on:
> > * double slashes
> > * trailing slash

> > I can do that. But at least on trailing slash looks to me quite strict.


> -1, trailing and double slash all accepted by shell in command line,
> maybe we shouldn't set a more strict policy than that.

Agree, I just didn't understand before your concern (you mostly objected the C
code, not the fact that the resulted path is modified).

Thanks for your reviewn!

Kind regards,
Petr




> > Whatever path we choose, I'd need also to update docs. BTW the need
> > to absolute path for TMPDIR is only in C - shell happily takes relative
> > path
> > and IMHO it's not documented.

> > Kind regards,
> > Petr

> > [1]
> > https://patchwork.ozlabs.org/project/ltp/patch/20230412073953.1983857-1-pvorel@suse.cz/

> > > >> -       if (!env_tmpdir)
> > > >> +       if (env_tmpdir) {
> > > >> +               /* remove duplicate slashes */
> > > >> +               for (char *p = env_tmpdir, *q = env_tmpdir; *q;) {
> > > >> +                       if (*++q != '/' || *p != '/')
> > > >> +                               *++p = *q;
> > > >> +               }
> > > >> +               /* Remove slash on the last place  */
> > > >> +               size_t last = strlen(env_tmpdir)-1;
> > > >> +               if (env_tmpdir[last] == '/')
> > > >> +                       env_tmpdir[last] = '\0';
> > > >> +       } else {
> > > >>                 env_tmpdir = TEMPDIR;
> > > >> +       }

> > > >>         if (env_tmpdir[0] != '/') {
> > > >>                 tst_brkm(TBROK, NULL, "You must specify an absolute "
> > > >>                                 "pathname for environment variable
> > > >> TMPDIR");
> > > >>                 return NULL;
> > > >>         }
> > > >> +
> > > >>         return env_tmpdir;
> > > >>  }

> > > >> --
> > > >> 2.40.0



> > > > --
> > > > Regards,
> > > > Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[Li Wang]

On Thu, Apr 20, 2023 at 10:53 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
> ...
> > > > >> +++ b/lib/tst_tmpdir.c
> > > > >> @@ -124,16 +124,28 @@ char *tst_get_tmpdir(void)
>
> > > > >>  const char *tst_get_tmpdir_root(void)
> > > > >>  {
> > > > >> -       const char *env_tmpdir = getenv("TMPDIR");
> > > > >> +       char *env_tmpdir = getenv("TMPDIR");
>
>
> > > > > It seems that modifying the environment variables is generally
> > > > > not a good practice.
>
> > > > > The getenv() function returns a pointer to the value of an
> > > > > environment variable, which is stored in the memory managed
> > > > > by the system. Any attempt to modify this memory directly can
> > > > > cause unexpected behavior or even crash the program.
>
> > > > > Instead of modifying the return value of getenv(), it is
> recommended
> > > > > to create a copy of the value and modify the copy instead.
>
> > > Do you mean to use strdup()?
>
>
> > Yeah, something like that, or we declare a buffer, and use strcpy()
> > to copy the string pointed to by the return value of getenv() into the
> > buffer that we can safely modify.
>
> > I prefer it in this way.
>
>
> Sure, I'll post new version with this. Until then I keep this patch open if
> anybody wants to comment it.
>
> > > Also man getenv(3) says:
>
> > >        As typically implemented, getenv() returns a pointer to a string
> > >        within the environment list.  The caller must take care not to
> > >        modify this string, since that would change the environment of
> > >        the process.
>
> > > => I would not mind $TMPDIR got updated in the environment.
>
> > > > Btw, the wise method is to use setenv() function to reset
> > > > environment variables if really needed.
>
> > > Well, I don't know any C test which needs it (only NFS tests which are
> > > shell
> > > tests). But I wanted to have the same behavior in both APIs.
>
> > > > This is a different part of shell API I have to say.
>
> > > Yes, the behavior is slightly different from shell API [1],
> > > where it modifies $TST_TMPDIR (keep $TMPDIR untouched).
>
>
> > > > > Or, the simplest way I guess is just TBROK and tell users why
> > > > > this TMPDIR is unusable.
>
> > > If you prefer it's better to TBROK on:
> > > * double slashes
> > > * trailing slash
>
> > > I can do that. But at least on trailing slash looks to me quite strict.
>
>
> > -1, trailing and double slash all accepted by shell in command line,
> > maybe we shouldn't set a more strict policy than that.
>
> Agree, I just didn't understand before your concern (you mostly objected
> the C
> code, not the fact that the resulted path is modified).
>

Yeah, I just doubted the incorrect way of doing that.
(in C programming)

Sorry for the unclear description, I'm always distressed by my English
spelling level :-(.


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] tst_tmpdir: Remove possible double/trailing slashes from TMPDIR

[Petr Vorel]

> On Thu, Apr 20, 2023 at 10:53 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Li,
> > ...
> > > > > >> +++ b/lib/tst_tmpdir.c
> > > > > >> @@ -124,16 +124,28 @@ char *tst_get_tmpdir(void)

> > > > > >>  const char *tst_get_tmpdir_root(void)
> > > > > >>  {
> > > > > >> -       const char *env_tmpdir = getenv("TMPDIR");
> > > > > >> +       char *env_tmpdir = getenv("TMPDIR");


> > > > > > It seems that modifying the environment variables is generally
> > > > > > not a good practice.

> > > > > > The getenv() function returns a pointer to the value of an
> > > > > > environment variable, which is stored in the memory managed
> > > > > > by the system. Any attempt to modify this memory directly can
> > > > > > cause unexpected behavior or even crash the program.

> > > > > > Instead of modifying the return value of getenv(), it is
> > recommended
> > > > > > to create a copy of the value and modify the copy instead.

> > > > Do you mean to use strdup()?


> > > Yeah, something like that, or we declare a buffer, and use strcpy()
> > > to copy the string pointed to by the return value of getenv() into the
> > > buffer that we can safely modify.

> > > I prefer it in this way.


> > Sure, I'll post new version with this. Until then I keep this patch open if
> > anybody wants to comment it.

> > > > Also man getenv(3) says:

> > > >        As typically implemented, getenv() returns a pointer to a string
> > > >        within the environment list.  The caller must take care not to
> > > >        modify this string, since that would change the environment of
> > > >        the process.

> > > > => I would not mind $TMPDIR got updated in the environment.

> > > > > Btw, the wise method is to use setenv() function to reset
> > > > > environment variables if really needed.

> > > > Well, I don't know any C test which needs it (only NFS tests which are
> > > > shell
> > > > tests). But I wanted to have the same behavior in both APIs.

> > > > > This is a different part of shell API I have to say.

> > > > Yes, the behavior is slightly different from shell API [1],
> > > > where it modifies $TST_TMPDIR (keep $TMPDIR untouched).


> > > > > > Or, the simplest way I guess is just TBROK and tell users why
> > > > > > this TMPDIR is unusable.

> > > > If you prefer it's better to TBROK on:
> > > > * double slashes
> > > > * trailing slash

> > > > I can do that. But at least on trailing slash looks to me quite strict.


> > > -1, trailing and double slash all accepted by shell in command line,
> > > maybe we shouldn't set a more strict policy than that.

> > Agree, I just didn't understand before your concern (you mostly objected
> > the C
> > code, not the fact that the resulted path is modified).


> Yeah, I just doubted the incorrect way of doing that.
> (in C programming)

> Sorry for the unclear description, I'm always distressed by my English
> spelling level :-(.

Don't worry, most of us are non-native speakers, thus the problem is sometimes
on the other side (me) :).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp