Fixed: regression in issue 1579 concerning readline() in d8.

src/d8.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 208 matching lines @@
     return ThrowException(String::New("Error loading file"));
   }
   return source;
 }
 
 
 Handle<Value> Shell::ReadLine(const Arguments& args) {
   static const int kBufferSize = 256;
   char buffer[kBufferSize];
   Handle<String> accumulator = String::New("");
-  bool linebreak;
   int length;
-  do {  // Repeat if the line ends with an escape '\'.
-    // fgets got an error. Just give up.
+  while (true) {
+    // Continue reading if the line ends with an escape '\\' or the line has
+    // not been fully read into the buffer yet (does not end with '\n').
+    // If fgets gets an error. Just give up.
     if (fgets(buffer, kBufferSize, stdin) == NULL) return Null();
     length = static_cast<int>(strlen(buffer));

[Sven Panne, 2011/08/01 11:25:06]

Using size_t for length gets rid of the cast. Furthermore, move declation into
this loop, keeping the scope as small as possible.

-    linebreak = (length > 1 && buffer[length-2] == '\\');
-    if (linebreak) buffer[length-2] = '\n';
-    accumulator = String::Concat(accumulator, String::New(buffer, length-1));
-  } while (linebreak);
-  return accumulator;
+    if (length == 0) return accumulator;

[Sven Panne, 2011/08/01 11:25:06]

In general, a simple if-then-else cascade is easier to read than nested logic,
e.g. in our concrete example:

if (length == 0) {
  // should never happen, but anyway...
  return accumulator;
} else if (buffer[length-1] != '\n') {
  // line doesn't fit into buffer, continue reading
  String::Concat(accumulator, String::New(buffer, length));
} else if (length >= 2 && buffer[length-2] == '\\') {
  // continuation line: replace backslash by newline and continue reading
  buffer[length-2] = '\n';
  String::Concat(accumulator, String::New(buffer, length - 1));
} else {
  // normal line, fitting into buffer
  return String::Concat(accumulator, String::New(buffer, length - 1));
}

+    if (buffer[length-1] == '\n') {
+      length--;
+      if (length == 0 || buffer[length-1] != '\\') {
+        return String::Concat(accumulator, String::New(buffer, length));
+      }
+      buffer[length-1] = '\n';
+    }
+    accumulator = String::Concat(accumulator, String::New(buffer, length));
+  }
 }
 
 
 Handle<Value> Shell::Load(const Arguments& args) {
   for (int i = 0; i < args.Length(); i++) {
     HandleScope handle_scope;
     String::Utf8Value file(args[i]);
     if (*file == NULL) {
       return ThrowException(String::New("Error loading file"));
     }
@@ skipping 1034 matching lines @@
 }
 
 }  // namespace v8
 
 
 #ifndef GOOGLE3
 int main(int argc, char* argv[]) {
   return v8::Shell::Main(argc, argv);
 }
 #endif