Better error reporting in nacl helper.

chrome/nacl/nacl_helper_linux.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // A mini-zygote specifically for Native Client.
 
 #include "chrome/common/nacl_helper_linux.h"
 
 #include <errno.h>
 #include <link.h>
@@ skipping 216 matching lines @@
     if (msglen == sizeof(kNaClForkRequest) - 1 &&
         memcmp(buf, kNaClForkRequest, msglen) == 0) {
       if (kNaClParentFDIndex + 1 == fds.size()) {
         HandleForkRequest(fds, prereserved_sandbox_size);
         continue;  // fork succeeded. Note: child does not return
       } else {
         LOG(ERROR) << "nacl_helper: unexpected number of fds, got "
                    << fds.size();
       }
     } else {
-      if (msglen != 0) {
-        LOG(ERROR) << "nacl_helper unrecognized request: %s";
+      if (msglen > 0) {

[Mark Seaborn, 2012/07/09 09:19:05]

">=" would be more appropriate than ">" since errno is only set on error, when
msglen < 0.

BTW, this code path would be clearer as:

if (msglen == 0 || (msglen == -1 && errno == ECONNRESET)) {
  ...
} else if (msglen < 0) {
  ...
} else if (msglen == sizeof...) {
  ...
} else {
  ...
}

i.e. known errors, unknown errors, known messages, unknown messages.

[halyavin, 2012/07/09 10:55:56]

Done.

+        LOG(ERROR) << "nacl_helper unrecognized request: "
+                   << std::string(buf, buf + msglen);

[Mark Seaborn, 2012/07/09 09:19:05]

Is this appropriate, given that the buffer might contain binary data?  If the
buffer contains all zeros (for example), this won't help much.  BTW, is there a
bug filed for the problem you are trying to track down?

[halyavin, 2012/07/09 10:55:56]

We will print zeros then. They will not be visible to the user, but we can ask
user to redirect output to the file and then see binary content using hex mode
in Midnight Commander.

There is bug filed. The problem with NaCl was reported by external user in NaCl
discuss. Do you think we should create one?

[Mark Seaborn, 2012/07/10 16:10:05]

That's kind of icky.  Maybe you could use GetDoubleQuotedJson() from
base/json/string_escape.h?
Yes, please file one referencing the thread & error, and link it from the commit
message.

         _exit(-1);
+      } else {
+        LOG(ERROR) << "receive from zygote failed, errno = " << errno;

[Mark Seaborn, 2012/07/09 09:19:05]

Prefix this with "nacl_helper:"?

[halyavin, 2012/07/09 10:55:56]

Done.

       }
     }
     // if fork fails, send PID=-1 to zygote
     if (!UnixDomainSocket::SendMsg(kNaClZygoteDescriptor, &badpid,
                                    sizeof(badpid), empty)) {
       LOG(ERROR) << "*** send() to zygote failed";
     }
   }
   CHECK(false);  // This routine must not return
 }