<div dir="ltr">All good - I just wanted to double check since I jumped the gun on Jason last time I took it the wrong way :-)</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 15, 2014 at 11:06 AM, Jim Ingham <span dir="ltr"><<a href="mailto:jingham@apple.com" target="_blank">jingham@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Sorry if I wasn’t clear, this looks fine.  Check it in.<span class="HOEnZb"><font color="#888888"><div><br></div><div>Jim</div></font></span><div><div class="h5"><div><br><div><div>On Sep 15, 2014, at 7:55 AM, Todd Fiala <<a href="mailto:tfiala@google.com" target="_blank">tfiala@google.com</a>> wrote:</div><br><blockquote type="cite"><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div dir="ltr">Hey Jim,<div><br></div><div>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?</div><div><br></div><div>-Todd</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 12, 2014 at 12:37 PM,<span> </span><span dir="ltr"><<a href="mailto:jingham@apple.com" target="_blank">jingham@apple.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">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.<br><br>Jim<br><div><div><br>> On Sep 12, 2014, at 11:15 AM, Shawn Best <<a href="mailto:sbest@blueshiftinc.com" target="_blank">sbest@blueshiftinc.com</a>> wrote:<br>><br>> Hi Jim, thanks for the feedback.<br>><br>> 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.<br>><br>> 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).<br>><br>> 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.<br>><br>> Yes, ProcessPosix, ProcessFreeBSD directly setting the variable, then calling a function to do so, looks a little weird, I've cleaned that up.<br>><br>> 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.<br>><br>> Shawn.<br>><br>><span> </span><a href="http://reviews.llvm.org/D5302" target="_blank">http://reviews.llvm.org/D5302</a><br>><br>> Files:<br>>  include/lldb/Core/Communication.h<br>>  include/lldb/Core/ConnectionFileDescriptor.h<br>>  include/lldb/Target/Process.h<br>>  source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp<br>>  source/Plugins/Process/POSIX/ProcessPOSIX.cpp<br>>  source/Plugins/Process/gdb-remote/ProcessGDBRemote.h<br>>  source/Target/Process.cpp<br>>  source/lldb-log.cpp<br></div></div>> <D5302.13649.patch>_______________________________________________<br><div><div>> lldb-commits mailing list<br>><span> </span><a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br>><span> </span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br><br></div></div></blockquote></div><br><br clear="all"><div><br></div>--<span> </span><br><div dir="ltr"><table cellspacing="0" cellpadding="0" style="color:rgb(136,136,136);font-family:'Times New Roman'"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Todd Fiala |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tfiala@google.com" style="color:rgb(17,85,204)" target="_blank"><span style="background-color:rgb(255,255,204);color:rgb(34,34,34)">tfiala@google.com</span></a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"><font color="#1155cc"> <a>650-943-3180</a></font></td></tr></tbody></table></div></div></div></blockquote></div><br></div></div></div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><table cellspacing="0" cellpadding="0" style="color:rgb(136,136,136);font-family:'Times New Roman'"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Todd Fiala |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tfiala@google.com" style="color:rgb(17,85,204)" target="_blank"><span style="background-color:rgb(255,255,204);color:rgb(34,34,34);background-repeat:initial initial">tfiala@google.com</span></a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"><font color="#1155cc"> <a>650-943-3180</a></font></td></tr></tbody></table><br></div>
</div>