Re: [PATCH] trace-cmd record: Reset PATH variable after strtok search

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Tue, 28 Nov 2023 15:22:19 -0500
Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:

> On 2023-11-28 15:18, Steven Rostedt wrote:
> > On Tue, 28 Nov 2023 14:13:02 -0600
> > David Vernet <void@xxxxxxxxxxxxx> wrote:  
> [...]
> >>> That would work too, though I don't think strtok_r() is doing anything
> >>> useful at that point. IMO it's better to either do the setenv() with
> >>> saveptr, or change that strtok_r() to a regular strtok().  
> > 
> > I always use strtok_r() over strtok() just because it's "safer"!
> > 
> > I know it's not necessary, but the number of times I had to switch it to
> > make the code thread safe, I just decided to always use it. Just my personal
> > preference.  
> And if you want to make your code thread-safe, you should favor working 
> on a strdup() copy rather than modifying the argv or env content.

Yes, the fix was to use strdup(). It wasn't even a thread issue, nor a
library issue, as the code in question is part of the trace-cmd executable
and not the libraries. The problem was that it modified the environment
variable and then reused that same environment variable in the exec()
operation :-p

> Also, modifying global state prevents code from being eventually re-used 
> in libraries.

That's why I now always use strtok_r() by default. It is thread (and
library) safe. I avoid using strtok() even when it's perfectly fine to do

-- Steve

[Index of Archives]     [Linux USB Development]     [Linux USB Development]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux