All good - I just wanted to double check since I jumped the gun on Jason last time I took it the wrong way :-)
On Mon, Sep 15, 2014 at 11:06 AM, Jim Ingham <[email protected]> wrote: > Sorry if I wasn’t clear, this looks fine. Check it in. > > Jim > > On Sep 15, 2014, at 7:55 AM, Todd Fiala <[email protected]> wrote: > > Hey Jim, > > Just for clarification, is this a LGTM? Or are you wanting Shawn to spend > more time looking at the bigger picture behind some of the locking? > > -Todd > > On Fri, Sep 12, 2014 at 12:37 PM, <[email protected]> wrote: > >> That looks okay. Thanks for looking at this. Seems to me that if you're >> only synchronization is locking access to some flag, then you're either >> doing something fairly weak (like the log example) or missing some real >> synchronization... So it's worth viewing the necessity of the change with >> skepticism. >> >> Jim >> >> > On Sep 12, 2014, at 11:15 AM, Shawn Best <[email protected]> >> wrote: >> > >> > Hi Jim, thanks for the feedback. >> > >> > I agree these may fall into the category of 'formally correct, but not >> strictly necessary.' One good thing is it makes clear the intention these >> variables are being directly accessed from multiple threads. >> > >> > Something I noticed about thread sanitizer is that it will complain if >> 2 threads access the same memory without a synchronization mechanism >> between them, even if those accesses are separated by a good amount of time >> (or some other kind of logic). >> > >> > Looking at closer at exit status, the problem, I think I should add a >> mutex to Process::SetExitStatus() , GetExitStatus(), GetExitDescription(). >> This should prevent an exit code and exit string potentially not matching. >> > >> > Yes, ProcessPosix, ProcessFreeBSD directly setting the variable, then >> calling a function to do so, looks a little weird, I've cleaned that up. >> > >> > As far as I can tell, if someone were to turn logging on/off in the >> middle of a function dumping log information... the worst that would happen >> is it would not start/stop a bit late. The typical usage pattern will >> check the flag once at the beginning of the function and keep the pointer >> for the duration of the function. I don't think that would be anything a >> user would notice. >> > >> > Shawn. >> > >> > http://reviews.llvm.org/D5302 >> > >> > Files: >> > include/lldb/Core/Communication.h >> > include/lldb/Core/ConnectionFileDescriptor.h >> > include/lldb/Target/Process.h >> > source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp >> > source/Plugins/Process/POSIX/ProcessPOSIX.cpp >> > source/Plugins/Process/gdb-remote/ProcessGDBRemote.h >> > source/Target/Process.cpp >> > source/lldb-log.cpp >> > <D5302.13649.patch>_______________________________________________ >> > lldb-commits mailing list >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits >> >> > > > -- > Todd Fiala | Software Engineer | [email protected] | 650-943-3180 > > > -- Todd Fiala | Software Engineer | [email protected] | 650-943-3180
_______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
