Added new ReadFileToString API with a max_size argument

base/file_util.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.
 
 #include "base/file_util.h"
 
 #if defined(OS_WIN)
 #include <io.h>
 #endif
 #include <stdio.h>
@@ skipping 108 matching lines @@
     else if (end2 + 1 < line2.length())
       line2.erase(end2 + 1);
 
     if (line1 != line2)
       return false;
   } while (!file1.eof() || !file2.eof());
 
   return true;
 }
 
-bool ReadFileToString(const FilePath& path, std::string* contents) {
-  if (path.ReferencesParent())
-    return false;
+static bool ReadFileToStringImpl(const FilePath& path, std::string* contents,
+                                 bool checkSz, size_t maxsize) {

[Andrew T Wilson (Slow), 2014/02/10 14:29:46]

nit: checkSz does not adhere to the style guidelines
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml). Should be
"check_size".

[bartfab (slow), 2014/02/10 14:55:37]

Further nit, also per style guide: Either put all arguments on one line, or put
each argument on a separate line. The middle ground is not allowed.

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

+  if(path.ReferencesParent())
+     return false;
   FILE* file = OpenFile(path, "rb");
-  if (!file) {
-    return false;
+  if(!file)
+     return false;

[Andrew T Wilson (Slow), 2014/02/10 14:29:46]

nit: space between if and ( and also make sure indent level is 2 characters, not
3.

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

+
+  if(!contents) {

[Andrew T Wilson (Slow), 2014/02/10 14:29:46]

space before open paren

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

+     CloseFile(file);

[Andrew T Wilson (Slow), 2014/02/10 14:29:46]

This breaks the old behavior of priming the disk cache when you pass a null
"contents", as documented in the comments for ReadFileToDisk().

[kaliamoorthi, 2014/02/11 10:10:45]

I changed the code to retain the old behavior of priming the disk cache. 

+     return true;
   }
 
   char buf[1 << 16];
-  size_t len;
+  size_t len, sz = 0;

[bartfab (slow), 2014/02/10 14:55:37]

Nit 1: Per style guide, avid abbreviations whenever possible. Thus, s/sz/size/

Nit 2: Do not declare more than one variable per line.

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

+
+  // Many files supplied in path have incorrect size (proc files, streams etc)
+  // hence the file is read sequentially as opposed to one-shot read

[Andrew T Wilson (Slow), 2014/02/10 14:29:46]

nit: period at the end of this sentence.

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

   while ((len = fread(buf, 1, sizeof(buf), file)) > 0) {
-    if (contents)
-      contents->append(buf, len);
+    sz += len;

[bartfab (slow), 2014/02/10 14:55:37]

Nit: What happens when |sz + len| overflows size_t?

This will never happen in practice for now because nobody has enough memory to
read a file of that size. But it is good to keep in mind that overflows can and
do occur.

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

+    if(checkSz && maxsize < sz) {

[Andrew T Wilson (Slow), 2014/02/10 14:29:46]

space before open paren.

Suggestion - instead of having a "checkSz" param, just treat maxsize = 0 as
"unlimited size". Or else have people pass in SIZE_MAX if they want an unlimited
size.

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

+      contents->clear();
+      CloseFile(file);
+      return false;
+    }
+    contents->append(buf, len);
   }
+
   CloseFile(file);
+  return true;
+}
 
-  return true;
+bool ReadFileToString(const FilePath& path, std::string* contents,
+                      size_t maxsize) {
+  return ReadFileToStringImpl(path, contents, true, maxsize);
+}
+
+bool ReadFileToString(const FilePath& path, std::string* contents) {
+  return ReadFileToStringImpl(path, contents, false, 0);

[Andrew T Wilson (Slow), 2014/02/10 14:29:46]

Suggest passing SIZE_MAX for an unlimited read, rather than having a separate
variable (see above).

[kaliamoorthi, 2014/02/11 10:10:45]

Done.

 }
 
 bool IsDirectoryEmpty(const FilePath& dir_path) {
   FileEnumerator files(dir_path, false,
       FileEnumerator::FILES | FileEnumerator::DIRECTORIES);
   if (files.Next().empty())
     return true;
   return false;
 }
 
@@ skipping 84 matching lines @@
     if (!PathExists(new_path) &&
         (!have_suffix || !PathExists(FilePath(new_path.value() + suffix)))) {
       return count;
     }
   }
 
   return -1;
 }
 
 }  // namespace file_util