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

[Petr Vorel <pvorel@suse.cz>]

> 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