Improve error handling in bspatch.

third_party/bspatch/mbspatch.cc

 /*-
  * Copyright 2003,2004 Colin Percival
  * All rights reserved
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted providing that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 193 matching lines @@
  * OR.  Note that this would be DANGEROUS AND UNSOUND if we used
  * _O_BINARY other than as a bitwise OR mask (e.g., as a bitwise AND
  * mask to check for binary mode), but it seems OK in the limited
  * context of the following small function. */
 #ifndef _O_BINARY
 # define _O_BINARY 0
 #endif
 
 int ApplyBinaryPatch(const wchar_t *old_file, const wchar_t *patch_file,
                      const wchar_t *new_file) {
-  int ret = 0;
+  int ret = OK;
+  int ofd = -1;
+  int nfd = -1;
+  unsigned char *buf = NULL;
+
   int pfd = _wopen(patch_file, O_RDONLY | _O_BINARY);
   if (pfd < 0) return READ_ERROR;
 
-  MBSPatchHeader header;
-  if (ret = MBS_ReadHeader(pfd, &header)) return ret;
+  do {
+    MBSPatchHeader header;
+    if ((ret = MBS_ReadHeader(pfd, &header)))
+      break;
 
-  int ofd = _wopen(old_file, O_RDONLY | _O_BINARY);
-  if (ofd < 0) return READ_ERROR;
+    ofd = _wopen(old_file, O_RDONLY | _O_BINARY);
+    if (ofd < 0) {
+      ret = READ_ERROR;
+      break;
+    }
 
-  struct stat os;
-  if (ret = fstat(ofd, &os)) return READ_ERROR;
-  if (os.st_size != header.slen) return UNEXPECTED_ERROR;
+    struct stat os;
+    if ((ret = fstat(ofd, &os))) {
+      ret = READ_ERROR;
+      break;
+    }
 
-  unsigned char *buf = (unsigned char*) malloc(header.slen);
-  if (!buf) return MEM_ERROR;
+    if (os.st_size != header.slen) {
+      ret = UNEXPECTED_ERROR;
+      break;
+    }
 
-  if (read(ofd, buf, header.slen) != header.slen) return READ_ERROR;
-  if (CalculateCrc(buf, header.slen) != header.scrc32) 
-    return CRC_ERROR;
+    buf = (unsigned char*) malloc(header.slen);
+    if (!buf) {
+      ret = MEM_ERROR;
+      break;
+    }
 
-  int nfd = _wopen(new_file, O_WRONLY | O_TRUNC | O_CREAT | _O_BINARY);
-  if (nfd < 0) return READ_ERROR;
+    if (read(ofd, buf, header.slen) != header.slen) {
+      ret = READ_ERROR;
+      break;
+    }
 
-  MBS_ApplyPatch(&header, pfd, buf, nfd);
+    if (CalculateCrc(buf, header.slen) != header.scrc32) {
+      ret = CRC_ERROR;
+      break;
+    }
+
+    nfd = _wopen(new_file, O_WRONLY | O_TRUNC | O_CREAT | _O_BINARY);
+    if (nfd < 0) {
+      ret = READ_ERROR;
+      break;
+    }
+
+    MBS_ApplyPatch(&header, pfd, buf, nfd);
+  } while (0);

[Avi (use Gerrit), 2013/07/29 14:46:21]

Huh. I don't think I've seen this style anywhere in Chrome. OTOH, while this is
C++, significantly altering the a third_party file to use RAII constructs is
probably undesirable.

 
   free(buf);
   close(pfd);
-  close(ofd);
-  close(nfd);
-  return OK;
+  if (ofd >= 0) close(ofd);
+  if (nfd >= 0) close(nfd);
+  return ret;
 }