Make string_util::WriteInto() DCHECK() that the supplied |length_with_null| > 1, meaning that the...

base/base_paths_mac.mm

 // Copyright (c) 2011 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.
 
 #include "base/base_paths_mac.h"
 
 #include <dlfcn.h>
 #import <Foundation/Foundation.h>
 #include <mach-o/dyld.h>
 
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/mac/foundation_util.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 
 namespace {
 
 bool GetNSExecutablePath(FilePath* path) WARN_UNUSED_RESULT;
 
 bool GetNSExecutablePath(FilePath* path) {
   DCHECK(path);
   // Executable path can have relative references ("..") depending on
   // how the app was launched.
   uint32_t executable_length = 0;
   _NSGetExecutablePath(NULL, &executable_length);
-  DCHECK_GE(executable_length, 1u);
+  DCHECK_GT(executable_length, 1u);

[cbentzel, 2011/11/15 11:37:10]

Is this guaranteed? Should "return false" be used if executable_length == 0?

[Peter Kasting, 2011/11/15 18:50:32]

The old code already DCHECKed that the length wasn't zero.  I'm just checking
that it isn't one either.  I can't see how the full path to the executable could
possibly contain one character (even if the executable itself were named
something one character long).

[Mark Mentovai, 2011/11/28 22:41:21]

Peter Kasting wrote:
The call you’re checking has a break-down case in which it might not return the
full path to the executable, but just the leaf filename. But it’s returning the
required buffer size with a terminator, so the executable would have to be empty
for executable_length to be 1, therefore this ought to be fine.

   std::string executable_path;
-  char* executable_path_c = WriteInto(&executable_path, executable_length);
-  int rv = _NSGetExecutablePath(executable_path_c, &executable_length);
+  int rv = _NSGetExecutablePath(WriteInto(&executable_path, executable_length),
+                                &executable_length);
   DCHECK_EQ(rv, 0);
-  DCHECK(!executable_path.empty());
-  if ((rv != 0) || (executable_path.empty()))
-    return false;
   *path = FilePath(executable_path);
   return true;

[Mark Mentovai, 2011/11/28 22:41:21]

As written now, is there even a good reason for this to have a bool return type?
That just seems to indicate that it might sometimes return false, but it never
does.

[Peter Kasting, 2011/11/29 01:48:15]

I agree with you.  Changed to void.

 }
 
 // Returns true if the module for |address| is found. |path| will contain
 // the path to the module. Note that |path| may not be absolute.
 bool GetModulePathForAddress(FilePath* path,
                              const void* address) WARN_UNUSED_RESULT;
 
 bool GetModulePathForAddress(FilePath* path, const void* address) {
   Dl_info info;
   if (dladdr(address, &info) == 0)
@@ skipping 35 matching lines @@
         *result = result->DirName().DirName();
       }
       return true;
     }
     default:
       return false;
   }
 }
 
 }  // namespace base