Fix "unreachable code" warnings (MSVC warning 4702) in net/.

net/spdy/spdy_network_transaction_unittest.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 <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
@@ skipping 4207 matching lines @@
 
   const HttpResponseInfo* response = trans->GetResponseInfo();
   EXPECT_TRUE(response->headers.get() != NULL);
   EXPECT_TRUE(response->was_fetched_via_spdy);
   out.status_line = response->headers->GetStatusLine();
   out.response_info = *response;  // Make a copy so we can verify.
 
   // Read Data
   TestCompletionCallback read_callback;
 
-  do {
-    const int kReadSize = 256;
-    scoped_refptr<net::IOBuffer> buf(new net::IOBuffer(kReadSize));
-    rv = trans->Read(buf.get(), kReadSize, read_callback.callback());
-    if (rv == net::ERR_IO_PENDING) {
-      // Complete the read now, which causes buffering to start.
-      data.CompleteRead();
-      // Destroy the transaction, causing the stream to get cancelled
-      // and orphaning the buffered IO task.
-      helper.ResetTrans();
-      break;
-    }
+  const int kReadSize = 256;
+  scoped_refptr<net::IOBuffer> buf(new net::IOBuffer(kReadSize));
+  rv = trans->Read(buf.get(), kReadSize, read_callback.callback());
+  if (rv != net::ERR_IO_PENDING) {
     // We shouldn't get here in this test.
     FAIL() << "Unexpected read: " << rv;

[rvargas (doing something else), 2014/03/19 22:13:15]

This just logs a message (and keeps track of the failure) as opposed to aborting
the test, right? Where is the old code unreachable?

I'm assuming that the change in logic (read only once vs read until pending) is
your way of saying if we're going to fail the test, why keep going (which sounds
right)

[Peter Kasting, 2014/03/19 22:17:09]

Wrong.  FAIL() returns from the function.  You're thinking of ADD_FAILURE().

[rvargas (doing something else), 2014/03/19 22:56:24]

OK then. I still don't see the unreachable line in the old version.

At this point I would do
ASSERT_EQ(net::ERR_IO_PENDING, rv) << "foo"

[Peter Kasting, 2014/03/19 23:03:24]

MSVC complains that the "while (rv > 0)" is an unreachable test.
Yeah, that is way better... dunno why I didn't think of that.  Done.

-  } while (rv > 0);
+  }
+  // Complete the read now, which causes buffering to start.
+  data.CompleteRead();

[rvargas (doing something else), 2014/03/19 22:13:15]

This could end up being called when rv is not pending. Sounds better to avoid
that.

[Peter Kasting, 2014/03/19 22:17:09]

No, it can't; see my reply above.

+  // Destroy the transaction, causing the stream to get cancelled
+  // and orphaning the buffered IO task.
+  helper.ResetTrans();
 
   // Flush the MessageLoop; this will cause the buffered IO task
   // to run for the final time.
   base::RunLoop().RunUntilIdle();
 
   // Verify that we consumed all test data.
   helper.VerifyDataConsumed();
 }
 
 // Test that if the server requests persistence of settings, that we save
@@ skipping 2251 matching lines @@
   // since we're send-stalled.
   EXPECT_TRUE(stream->stream()->send_stalled_by_flow_control());
 
   // Read in WINDOW_UPDATE or SETTINGS frame.
   data.RunFor((GetParam().protocol >= kProtoSPDY31) ? 8 : 7);
   rv = callback.WaitForResult();
   helper.VerifyDataConsumed();
 }
 
 }  // namespace net