More code cleanups

src/uri.cc

@@ -105,65 +105,76 @@ bool parse(UriStruct& result, const std::string& uri)
   int rv;
   const char* p = uri.c_str();
   rv = uri_split(&res, p);
-  if(rv == 0) {
-    result.protocol.assign(p + res.fields[USR_SCHEME].off,
-                           res.fields[USR_SCHEME].len);
-    result.host.assign(p + res.fields[USR_HOST].off, res.fields[USR_HOST].len);
-    if(res.port == 0) {
-      uint16_t defPort;
-      if((defPort = getDefaultPort(result.protocol)) == 0) {
-        return false;
-      }
-      result.port = defPort;
-    } else {
-      result.port = res.port;
+  if(rv != 0) {
+    return false;
+  }
+
+  result.protocol.assign(p + res.fields[USR_SCHEME].off,
+                         res.fields[USR_SCHEME].len);
+  result.host.assign(p + res.fields[USR_HOST].off, res.fields[USR_HOST].len);
+  if (res.port == 0) {
+    uint16_t defPort;
+    if((defPort = getDefaultPort(result.protocol)) == 0) {
+      return false;
     }
-    if(res.field_set & (1 << USR_PATH)) {
-      if(res.field_set & (1 << USR_BASENAME)) {
-        result.dir.assign(p + res.fields[USR_PATH].off,
-                          res.fields[USR_PATH].len -
+    result.port = defPort;
+  }
+  else {
+    result.port = res.port;
+  }
+
+  if(res.field_set & (1 << USR_PATH)) {
+    if(res.field_set & (1 << USR_BASENAME)) {
+      result.dir.assign(p + res.fields[USR_PATH].off,
+                        res.fields[USR_PATH].len -
+                        res.fields[USR_BASENAME].len);
+      result.file.assign(p + res.fields[USR_BASENAME].off,
                           res.fields[USR_BASENAME].len);
-        result.file.assign(p + res.fields[USR_BASENAME].off,
-                           res.fields[USR_BASENAME].len);
-      } else {
-        result.dir.assign(p + res.fields[USR_PATH].off,
-                          res.fields[USR_PATH].len);
-        result.file = A2STR::NIL;
-      }
-    } else {
-      result.dir = "/";
-      result.file = A2STR::NIL;
-    }
-    if(res.field_set & (1 << USR_QUERY)) {
-      result.query = "?";
-      result.query.append(p + res.fields[USR_QUERY].off,
-                          res.fields[USR_QUERY].len);
-    } else {
-      result.query = A2STR::NIL;
-    }
-    if(res.field_set & (1 << USR_USER)) {
-      result.username.assign(p + res.fields[USR_USER].off,
-                             res.fields[USR_USER].len);
-      result.username = util::percentDecode(result.username.begin(),
-                                            result.username.end());
-    } else {
-      result.username = A2STR::NIL;
     }
-    if(res.field_set & (1 << USR_PASSWD)) {
-      result.hasPassword = true;
-      result.password.assign(p + res.fields[USR_PASSWD].off,
-                             res.fields[USR_PASSWD].len);
-      result.password = util::percentDecode(result.password.begin(),
-                                            result.password.end());
-    } else {
-      result.hasPassword = false;
-      result.password = A2STR::NIL;
+    else {
+      result.dir.assign(p + res.fields[USR_PATH].off,
+                        res.fields[USR_PATH].len);
+      result.file = A2STR::NIL;
     }
-    result.ipv6LiteralAddress = res.flags & USF_IPV6ADDR;
-    return true;
-  } else {
-    return false;
   }
+  else {
+    result.dir = "/";
+    result.file = A2STR::NIL;
+  }
+
+  if(res.field_set & (1 << USR_QUERY)) {
+    result.query = "?";
+    result.query.append(p + res.fields[USR_QUERY].off,
+                        res.fields[USR_QUERY].len);
+  }
+  else {
+    result.query = A2STR::NIL;
+  }
+
+  if(res.field_set & (1 << USR_USER)) {
+    result.username.assign(p + res.fields[USR_USER].off,
+                            res.fields[USR_USER].len);
+    result.username = util::percentDecode(result.username.begin(),
+                                          result.username.end());
+  }
+  else {
+    result.username = A2STR::NIL;
+  }
+
+  if(res.field_set & (1 << USR_PASSWD)) {
+    result.hasPassword = true;
+    result.password.assign(p + res.fields[USR_PASSWD].off,
+                            res.fields[USR_PASSWD].len);
+    result.password = util::percentDecode(result.password.begin(),
+                                          result.password.end());
+  }
+  else {
+    result.hasPassword = false;
+    result.password = A2STR::NIL;
+  }
+
+  result.ipv6LiteralAddress = res.flags & USF_IPV6ADDR;
+  return true;
 }
 
 std::string getFieldString(const uri_split_result& res, int field,
@@ -171,9 +182,8 @@ std::string getFieldString(const uri_split_result& res, int field,
 {
   if(res.field_set & (1 << field)) {
     return std::string(base + res.fields[field].off, res.fields[field].len);
-  } else {
-    return "";
   }
+  return "";
 }
 
 std::string construct(const UriStruct& us)
@@ -193,39 +203,48 @@ std::string construct(const UriStruct& us)
     res += "[";
     res += us.host;
     res += "]";
-  } else {
+  }
+  else {
     res += us.host;
   }
-  uint16_t defPort= getDefaultPort(us.protocol);
+
+  uint16_t defPort = getDefaultPort(us.protocol);
   if(us.port != 0 && defPort != us.port) {
     res += fmt(":%u", us.port);
   }
+
   res += us.dir;
   if(us.dir.empty() || us.dir[us.dir.size()-1] != '/') {
     res += "/";
   }
+
   res += us.file;
   res += us.query;
   return res;
 }
 
-enum {
-  NPATH_START,
-  NPATH_SLASH,
-  NPATH_SDOT,
-  NPATH_DDOT,
-  NPATH_PATHCOMP
-};
+namespace {
+  enum {
+    NPATH_START,
+    NPATH_SLASH,
+    NPATH_SDOT,
+    NPATH_DDOT,
+    NPATH_PATHCOMP
+  };
+}
 
 std::string normalizePath(std::string path)
 {
-  std::string::iterator begin = path.begin(), out = begin;
+  typedef std::string::iterator itr;
+
+  itr begin = path.begin(), out = begin;
   int state = NPATH_START;
   bool startWithSlash = false;
   std::vector<int> range;
   // 32 is arbitrary
   range.reserve(32);
-  for(std::string::iterator in = begin, eoi = path.end(); in != eoi; ++in) {
+
+  for(itr in = begin, eoi = path.end(); in != eoi; ++in) {
     switch(state) {
     case NPATH_START:
       switch(*in) {
@@ -295,6 +314,7 @@ std::string normalizePath(std::string path)
       break;
     }
   }
+
   switch(state) {
   case NPATH_SDOT:
     range.pop_back();
@@ -310,18 +330,21 @@ std::string normalizePath(std::string path)
   default:
     break;
   }
+
   if(startWithSlash) {
     ++out;
   }
+
   for(int i = 0; i < (int)range.size(); i += 2) {
-    std::string::iterator a = begin+range[i];
-    std::string::iterator b = begin+range[i+1];
+    itr a = begin + range[i];
+    itr b = begin + range[i+1];
     if(a == out) {
       out = b;
     } else {
       out = std::copy(a, b, out);
     }
   }
+
   path.erase(out, path.end());
   return path;
 }
@@ -333,16 +356,20 @@ std::string joinPath(std::string basePath,
 {
   if(newPathFirst == newPathLast) {
     return basePath;
-  } else if(basePath.empty() || *newPathFirst == '/') {
+  }
+
+  if(basePath.empty() || *newPathFirst == '/') {
     return normalizePath(std::string(newPathFirst, newPathLast));
-  } else if(basePath[basePath.size()-1] == '/') {
-    basePath.append(newPathFirst, newPathLast);
-    return normalizePath(basePath);
-  } else {
-    basePath += "/";
+  }
+
+  if(basePath[basePath.size()-1] == '/') {
     basePath.append(newPathFirst, newPathLast);
     return normalizePath(basePath);
   }
+
+  basePath += "/";
+  basePath.append(newPathFirst, newPathLast);
+  return normalizePath(basePath);
 }
 } // namespace
 
@@ -356,36 +383,37 @@ std::string joinUri(const std::string& baseUri, const std::string& uri)
   UriStruct us;
   if(parse(us, uri)) {
     return uri;
-  } else {
-    UriStruct bus;
-    if(!parse(bus, baseUri)) {
-      return uri;
-    }
-    std::string::const_iterator qend;
-    for(qend = uri.begin(); qend != uri.end(); ++qend) {
-      if(*qend == '#') {
-        break;
-      }
-    }
-    std::string::const_iterator end;
-    for(end = uri.begin(); end != qend; ++end) {
-      if(*end == '?') {
-        break;
-      }
+  }
+
+  UriStruct bus;
+  if(!parse(bus, baseUri)) {
+    return uri;
+  }
+
+  std::string::const_iterator qend;
+  for(qend = uri.begin(); qend != uri.end(); ++qend) {
+    if(*qend == '#') {
+      break;
     }
-    std::string newpath = joinPath(bus.dir, uri.begin(), end);
-    bus.dir.clear();
-    bus.file.clear();
-    bus.query.clear();
-    std::string res = construct(bus);
-    if(!newpath.empty()) {
-      // res always ends with '/'. Since bus.dir also starts with '/',
-      // regardless of uri, newpath always starts with '/'.
-      res.append(newpath.begin()+1, newpath.end());
+  }
+  std::string::const_iterator end;
+  for(end = uri.begin(); end != qend; ++end) {
+    if(*end == '?') {
+      break;
     }
-    res.append(end, qend);
-    return res;
   }
+  std::string newpath = joinPath(bus.dir, uri.begin(), end);
+  bus.dir.clear();
+  bus.file.clear();
+  bus.query.clear();
+  std::string res = construct(bus);
+  if(!newpath.empty()) {
+    // res always ends with '/'. Since bus.dir also starts with '/',
+    // regardless of uri, newpath always starts with '/'.
+    res.append(newpath.begin()+1, newpath.end());
+  }
+  res.append(end, qend);
+  return res;
 }
 
 } // namespace uri

src/util.cc

@@ -279,8 +279,8 @@ bool isHexDigit(const char c)
 
 bool isHexDigit(const std::string& s)
 {
-  for(std::string::const_iterator i = s.begin(), eoi = s.end(); i != eoi; ++i) {
-    if(!isHexDigit(*i)) {
+  for (const auto& c : s) {
+    if(!isHexDigit(c)) {
       return false;
     }
   }
@@ -676,15 +676,15 @@ void computeHeadPieces
   if(head == 0) {
     return;
   }
-  for(auto fi = fileEntries.begin(), eoi = fileEntries.end(); fi != eoi; ++fi) {
-    if((*fi)->getLength() == 0) {
+  for (const auto& fi: fileEntries) {
+    if(fi->getLength() == 0) {
       continue;
     }
-    size_t lastIndex =
-      ((*fi)->getOffset()+std::min(head, (*fi)->getLength())-1)/pieceLength;
-    for(size_t index = (*fi)->getOffset()/pieceLength;
-        index <= lastIndex; ++index) {
-      indexes.push_back(index);
+    const size_t lastIndex = (fi->getOffset() +
+        std::min(head, fi->getLength()) - 1
+        ) / pieceLength;
+    for(size_t idx = fi->getOffset() / pieceLength; idx <= lastIndex; ++idx) {
+      indexes.push_back(idx);
     }
   }
 }
@@ -700,16 +700,16 @@ void computeTailPieces
   if(tail == 0) {
     return;
   }
-  for(auto fi = fileEntries.begin(), eoi = fileEntries.end(); fi != eoi; ++fi) {
-    if((*fi)->getLength() == 0) {
+  for (const auto& fi: fileEntries) {
+    if(fi->getLength() == 0) {
       continue;
     }
-    int64_t endOffset = (*fi)->getLastOffset();
-    size_t fromIndex =
-      (endOffset-1-(std::min(tail, (*fi)->getLength())-1))/pieceLength;
-    for(size_t index = fromIndex; index <= (endOffset-1)/pieceLength;
-        ++index) {
-      indexes.push_back(index);
+    int64_t endOffset = fi->getLastOffset();
+    size_t fromIndex = (endOffset - 1 - (std::min(tail, fi->getLength()) - 1)) /
+      pieceLength;
+    const size_t toIndex = (endOffset - 1) / pieceLength;
+    while (fromIndex <= toIndex) {
+      indexes.push_back(fromIndex++);
     }
   }
 }
@@ -724,23 +724,26 @@ void parsePrioritizePieceRange
   std::vector<size_t> indexes;
   std::vector<Scip> parts;
   splitIter(src.begin(), src.end(), std::back_inserter(parts), ',', true);
-  for(std::vector<Scip>::const_iterator i = parts.begin(),
-        eoi = parts.end(); i != eoi; ++i) {
-    if(util::streq((*i).first, (*i).second, "head")) {
+  for (const auto& i: parts) {
+    if(util::streq(i.first, i.second, "head")) {
       computeHeadPieces(indexes, fileEntries, pieceLength, defaultSize);
-    } else if(util::startsWith((*i).first, (*i).second, "head=")) {
-      std::string sizestr((*i).first+5, (*i).second);
+    }
+    else if(util::startsWith(i.first, i.second, "head=")) {
+      std::string sizestr(i.first + 5, i.second);
       computeHeadPieces(indexes, fileEntries, pieceLength,
                         std::max((int64_t)0, getRealSize(sizestr)));
-    } else if(util::streq((*i).first, (*i).second, "tail")) {
+    }
+    else if(util::streq(i.first, i.second, "tail")) {
       computeTailPieces(indexes, fileEntries, pieceLength, defaultSize);
-    } else if(util::startsWith((*i).first, (*i).second, "tail=")) {
-      std::string sizestr((*i).first+5, (*i).second);
+    }
+    else if(util::startsWith(i.first, i.second, "tail=")) {
+      std::string sizestr(i.first + 5, i.second);
       computeTailPieces(indexes, fileEntries, pieceLength,
                         std::max((int64_t)0, getRealSize(sizestr)));
-    } else {
+    }
+    else {
       throw DL_ABORT_EX(fmt("Unrecognized token %s",
-                            std::string((*i).first, (*i).second).c_str()));
+                            std::string(i.first, i.second).c_str()));
     }
   }
   std::sort(indexes.begin(), indexes.end());
@@ -1177,20 +1180,19 @@ std::string getContentDispositionFilename(const std::string& header)
                                          header.c_str(), header.size());
   if(rv == -1) {
     return "";
+  }
+
+  std::string res;
+  if(!charset || strieq(charset, charset+charsetlen, "iso-8859-1")) {
+    res = iso8859p1ToUtf8(cdval, rv);
   } else {
-    std::string res;
-    if(!charset || strieq(charset, charset+charsetlen, "iso-8859-1")) {
-      res = iso8859p1ToUtf8(cdval, rv);
-    } else {
-      res.assign(cdval, rv);
-    }
-    if(!detectDirTraversal(res) &&
-       res.find_first_of("/\\") == std::string::npos) {
-      return res;
-    } else {
-      return "";
-    }
+    res.assign(cdval, rv);
   }
+  if(!detectDirTraversal(res) &&
+      res.find_first_of("/\\") == std::string::npos) {
+    return res;
+  }
+  return "";
 }
 
 std::string toUpper(std::string src)
@@ -1335,32 +1337,30 @@ void setGlobalSignalHandler(int sig, sigset_t* mask, signal_handler_t handler,
 std::string getHomeDir()
 {
   const char* p = getenv("HOME");
-  if(p) {
+  if (p) {
     return p;
-  } else {
+  }
 #ifdef __MINGW32__
-    p = getenv("USERPROFILE");
-    if(p) {
-      return p;
-    } else {
-      p = getenv("HOMEDRIVE");
-      if(p) {
-        std::string homeDir = p;
-        p = getenv("HOMEPATH");
-        if(p) {
-          homeDir += p;
-          return homeDir;
-        }
-      }
+  p = getenv("USERPROFILE");
+  if (p) {
+    return p;
+  }
+  p = getenv("HOMEDRIVE");
+  if (p) {
+    std::string homeDir = p;
+    p = getenv("HOMEPATH");
+    if (p) {
+      homeDir += p;
+      return homeDir;
     }
+  }
 #elif HAVE_PWD_H
-    passwd* pw = getpwuid(geteuid());
-    if(pw && pw->pw_dir) {
-      return pw->pw_dir;
-    }
-#endif // HAVE_PWD_H
-    return A2STR::NIL;
+  passwd* pw = getpwuid(geteuid());
+  if(pw && pw->pw_dir) {
+    return pw->pw_dir;
   }
+#endif // HAVE_PWD_H
+  return A2STR::NIL;
 }
 
 int64_t getRealSize(const std::string& sizeWithUnit)
@@ -1387,7 +1387,8 @@ int64_t getRealSize(const std::string& sizeWithUnit)
   if(!parseLLIntNoThrow(v, size) || v < 0) {
     throw DL_ABORT_EX(fmt("Bad or negative value detected: %s",
                           sizeWithUnit.c_str()));
-  } else if(INT64_MAX/mult < v) {
+  }
+  if(INT64_MAX/mult < v) {
     throw DL_ABORT_EX(fmt(MSG_STRING_INTEGER_CONVERSION_FAILURE,
                           "overflow/underflow"));
   }
@@ -1474,11 +1475,9 @@ void mkdirs(const std::string& dirpath)
   if(!dir.mkdirs()) {
     int errNum = errno;
     if(!dir.isDir()) {
-      throw DL_ABORT_EX3
-        (errNum,
-         fmt(EX_MAKE_DIR, dir.getPath().c_str(),
-             safeStrerror(errNum).c_str()),
-         error_code::DIR_CREATE_ERROR);
+      throw DL_ABORT_EX3(errNum, fmt(EX_MAKE_DIR, dir.getPath().c_str(),
+                                     safeStrerror(errNum).c_str()),
+                         error_code::DIR_CREATE_ERROR);
     }
   }
 }
@@ -1518,9 +1517,8 @@ void* allocateAlignedMemory(size_t alignment, size_t size)
   void* buffer;
   int res;
   if((res = posix_memalign(&buffer, alignment, size)) != 0) {
-    throw FATAL_EXCEPTION
-      (fmt("Error in posix_memalign: %s",
-           util::safeStrerror(res).c_str()));
+    throw FATAL_EXCEPTION(fmt("Error in posix_memalign: %s",
+                              util::safeStrerror(res).c_str()));
   }
   return buffer;
 }
@@ -1663,11 +1661,13 @@ std::string applyDir(const std::string& dir, const std::string& relPath)
   if(dir.empty()) {
     s = "./";
     s += relPath;
-  } else {
+  }
+  else {
     s = dir;
     if(dir == "/") {
       s += relPath;
-    } else {
+    }
+    else {
       s += "/";
       s += relPath;
     }
@@ -1728,18 +1728,19 @@ bool detectDirTraversal(const std::string& s)
   if(s.empty()) {
     return false;
   }
-  for(std::string::const_iterator i = s.begin(), eoi = s.end(); i != eoi; ++i) {
-    unsigned char c = *i;
-    if(in(c, 0x00u, 0x1fu) || c == 0x7fu) {
+  for (const auto& ch: s) {
+    if (in(ch, 0x00u, 0x1fu) || ch == 0x7fu) {
       return true;
     }
   }
   return s == "." || s == ".." || s[0] == '/' ||
-    util::startsWith(s, "./") || util::startsWith(s, "../") ||
+    util::startsWith(s, "./") ||
+    util::startsWith(s, "../") ||
     s.find("/../") != std::string::npos ||
     s.find("/./") != std::string::npos ||
     s[s.size()-1] == '/' ||
-    util::endsWith(s, "/.") || util::endsWith(s, "/..");
+    util::endsWith(s, "/.") ||
+    util::endsWith(s, "/..");
 }
 
 std::string escapePath(const std::string& s)
@@ -1810,9 +1811,8 @@ void executeHook
                   numFilesStr.c_str(),
                   firstFilename.c_str()));
   pid_t cpid = fork();
-  if(cpid == -1) {
-    A2_LOG_ERROR("fork() failed. Cannot execute user command.");
-  } else if(cpid == 0) {
+  if (cpid > 0) {
+    // child!
     execlp(command.c_str(),
            command.c_str(),
            gidStr.c_str(),
@@ -1821,8 +1821,15 @@ void executeHook
            reinterpret_cast<char*>(0));
     perror(("Could not execute user command: "+command).c_str());
     _exit(EXIT_FAILURE);
+    return;
   }
-#else
+
+  if(cpid == -1) {
+    A2_LOG_ERROR("fork() failed. Cannot execute user command.");
+  }
+  return;
+
+#else // __MINGW32__
   PROCESS_INFORMATION pi;
   STARTUPINFOW si;
 
@@ -1832,6 +1839,8 @@ void executeHook
   bool batch = util::iendsWith(command, ".bat");
   std::string cmdline;
   std::string cmdexe;
+
+  // XXX batch handling, in particular quoting, correct?
   if(batch) {
     const char* p = getenv("windir");
     if(p) {
@@ -1878,6 +1887,8 @@ void executeHook
   if(!rc) {
     A2_LOG_ERROR("CreateProcess() failed. Cannot execute user command.");
   }
+  return;
+
 #endif
 }
 
@@ -1912,11 +1923,10 @@ void executeHookByOptName
 std::string createSafePath
 (const std::string& dir, const std::string& filename)
 {
-  return util::applyDir
-    (dir,
-     util::isUtf8(filename)?
-     util::fixTaintedBasename(filename):
-     util::escapePath(util::percentEncode(filename)));
+  return util::applyDir(dir, util::isUtf8(filename) ?
+                             util::fixTaintedBasename(filename) :
+                             util::escapePath(util::percentEncode(filename))
+                        );
 }
 
 std::string encodeNonUtf8(const std::string& s)
@@ -1926,11 +1936,10 @@ std::string encodeNonUtf8(const std::string& s)
 
 std::string makeString(const char* str)
 {
-  if(str) {
-    return str;
-  } else {
+  if(!str) {
     return A2STR::NIL;
   }
+  return str;
 }
 
 std::string safeStrerror(int errNum)
@@ -1944,9 +1953,8 @@ bool noProxyDomainMatch
 {
   if(!domain.empty() && domain[0] == '.' && !util::isNumericHost(hostname)) {
     return util::endsWith(hostname, domain);
-  } else {
-    return hostname == domain;
   }
+  return hostname == domain;
 }
 
 bool tlsHostnameMatch(const std::string& pattern, const std::string& hostname)
@@ -1987,11 +1995,10 @@ bool tlsHostnameMatch(const std::string& pattern, const std::string& hostname)
   if(hnLeftLabelEnd - hostname.begin() < ptLeftLabelEnd - pattern.begin()) {
     return false;
   }
-  return
-    istartsWith(hostname.begin(), hnLeftLabelEnd,
-                pattern.begin(), ptWildcard) &&
-    iendsWith(hostname.begin(), hnLeftLabelEnd,
-              ptWildcard+1, ptLeftLabelEnd);
+  return istartsWith(hostname.begin(), hnLeftLabelEnd, pattern.begin(),
+                     ptWildcard) &&
+         iendsWith(hostname.begin(), hnLeftLabelEnd, ptWildcard + 1,
+                   ptLeftLabelEnd);
 }
 
 bool strieq(const std::string& a, const char* b)