bearjb / gperftools

Automatically exported from code.google.com/p/gperftools
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Early exit from a loop in socketpair.cc #658

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In the following loop in socketpair.cc 
(https://code.google.com/p/gperftools/codesearch#gperftools/trunk/src/symbolize.
cc&q=socketpair&sq=package:gperftools&l=148):
-------------------------------------
  for (int i = 0; i < 5; i++) {
    if (socketpair(AF_UNIX, SOCK_STREAM, 0, child_fds[i]) == -1) {
      for (int j = 0; j < i; j++) {
        close(child_fds[j][0]);
        close(child_fds[j][1]);
        PrintError("Cannot create a socket pair");
        return 0;
      }
    }
-------------------------------------
the return statement is misplaced, so the inner loop body is always executed 
exactly once.

This error has been spotted by PVS-studio (see 
https://code.google.com/p/chromium/issues/detail?id=324725)

The patch below should fix this issue:
-------------------------------------
diff --git a/src/symbolize.cc b/src/symbolize.cc
index 3530fca..a27106e 100755
--- a/src/symbolize.cc
+++ b/src/symbolize.cc
@@ -156,8 +156,8 @@ int SymbolTable::Symbolize() {
         close(child_fds[j][0]);
         close(child_fds[j][1]);
         PrintError("Cannot create a socket pair");
-        return 0;
       }
+      return 0;
     } else {
       if ((child_fds[i][0] > 2) && (child_fds[i][1] > 2)) {
         if (child_in == NULL) {
-------------------------------------

Original issue reported on code.google.com by gli...@chromium.org on 24 Nov 2014 at 9:45

GoogleCodeExporter commented 9 years ago
I think PrintError could be moved out as well. But applying anyways. Thanks.

Original comment by alkondratenko on 27 Nov 2014 at 6:44

GoogleCodeExporter commented 9 years ago
Merged. Thanks.

Original comment by alkondratenko on 27 Nov 2014 at 6:46

GoogleCodeExporter commented 9 years ago
Agreed, it really makes sense to move PrintError out.

Original comment by gli...@chromium.org on 28 Nov 2014 at 1:53