Hi,<div><br></div><div><div class="gmail_extra"><div class="gmail_quote">On Fri, Dec 7, 2012 at 2:21 PM, Daniel Malea <span dir="ltr"><<a href="mailto:daniel.malea@intel.com" target="_blank">daniel.malea@intel.com</a>></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">...</blockquote><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">
- change sizeof to use a type name instead of variable name<br>
<br></blockquote><div><br></div><div>What, why? Using a variable name is usually what you want (and much safer, if said variable changes type). This doesn't apply here (we're setting what dst points to, not dst itself), but this sentence without any context felt weird. :-)</div>
<div><br></div><div> </div><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">

Modified: lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp?rev=169645&r1=169644&r2=169645&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp?rev=169645&r1=169644&r2=169645&view=diff</a><br>


==============================================================================<br>
--- lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp (original)<br>
+++ lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp Fri Dec  7 16:21:08 2012<br>
@@ -13,6 +13,7 @@<br>
 #include <errno.h><br>
 #include <poll.h><br>
 #include <string.h><br>
+#include <stdint.h><br>
 #include <unistd.h><br>
 #include <sys/ptrace.h><br>
 #include <sys/socket.h><br>
@@ -200,7 +201,7 @@<br>
<br>
         // Copy the data into our buffer<br>
         if (log)<br>
-            memset(dst, 0, sizeof(dst));<br>
+            memset(dst, 0, sizeof(unsigned char));<br>
         for (unsigned i = 0; i < remainder; ++i)<br>
             dst[i] = ((data >> i*8) & 0xFF);<br>
<br></blockquote><div><br></div><div><br></div><div>Here! We're only setting one of the bytes on dst. This doesn't make sense, to me.</div><div>It also took me some time to figure out why we're only setting dst when logging, but it's because a few lines ahead we do:</div>
<div><br></div><div><div>        if (log && ProcessPOSIXLog::AtTopNestLevel() &&</div><div>            (log->GetMask().Test(POSIX_LOG_MEMORY_DATA_LONG) ||</div><div>             (log->GetMask().Test(POSIX_LOG_MEMORY_DATA_SHORT) &&</div>
<div>              size <= POSIX_LOG_MEMORY_SHORT_BYTES)))</div><div>            log->Printf ("ProcessMonitor::%s() [%p]:0x%lx (0x%lx)", __FUNCTION__,</div><div>                         (void*)vm_addr, *(unsigned long*)dst, (unsigned long)data);</div>
</div><div><br></div><div>printing dst as if it were an unsigned long *.</div><div><br></div><div>I'm not familiar with the code on this part, but I would say that we want one of these:</div><div>  - change the memset size to sizeof(unsigned long*)</div>
<div>  - change the memset size to size, which I suppose is the size we have in the memory pointed to by buf (which gets static_cast'ed to dst). Are we always sure size is equal or greater than sizeof(unsigned long)?</div>
<div><br></div><div>Or maybe I didn't understand this part of the code.</div><div>Could someone more familiar with ProcessMonitor chip in?</div><div><br></div><div>Regards,</div><div><br></div><div>  Filipe</div><div>
<br></div></div></div></div>