[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