<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Apr 26, 2016 at 1:20 PM, Davide Italiano <span dir="ltr"><<a href="mailto:dccitaliano@gmail.com" target="_blank">dccitaliano@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">davide added inline comments.<br>
<span class=""><br>
================<br>
Comment at: ELF/Driver.cpp:258<br>
@@ -243,6 +257,3 @@<br>
<br>
-static void dumpLinkerInvocation(ArrayRef<const char *> Args) {<br>
-  std::error_code EC = sys::fs::create_directories(Config->Reproduce,<br>
-    false /* IgnoreExisting */);<br>
-  if (EC) {<br>
-    error(EC, "--reproduce: can't create directory");<br>
+static void logCommandline(ArrayRef<const char *> Args) {<br>
+  if (std::error_code EC = sys::fs::create_directories(<br>
----------------<br>
</span><span class="">ruiu wrote:<br>
> davide wrote:<br>
> > I prefer dumpLinkerInvocation still.<br>
> To me this name is more plain and therefore good. It just logs command line -- I can tell it would obviously have no side effect (except writing a log somewhere) just by looking at the name. But dumpLinkerInvocation sounds something more complex.<br>
</span>As you wish. Discussing this is probably not worth more time.<br>
<span class=""><br>
================<br>
Comment at: ELF/Driver.cpp:272<br>
@@ -257,3 +271,3 @@<br>
   OS << Args[0];<br>
-  for (unsigned I = 1, E = Args.size(); I < E; ++I)<br>
+  for (size_t I = 1, E = Args.size(); I < E; ++I)<br>
     OS << " " << Args[I];<br>
----------------<br>
</span><span class="">ruiu wrote:<br>
> davide wrote:<br>
> > We use unsigned everywhere else in the code base, try to be consistent or change all the other occurrences.<br>
> I don't think we do. I actually use size_t where it means the size of an array. This is also technically accurate (even though it is unrealistic that we have more than 2^32 arguments).<br>
</span>I think these should be changed then:<br>
<br>
Writer.cpp:  for (unsigned I = 0; I < OutputSections.size(); ++I) {<br>
OutputSections.cpp:    for (unsigned I = 0, N = Offsets.size(); I != N; ++I) {<br>
<br>
I don't care which one we use just try to use the same everywhere. Feel free to change in another patch.<br></blockquote><div><br></div><div>Sure, I'll probably do in a follow-up patch. </div></div></div></div>