lib: Optimize file_compare
authorVolker Lendecke <vl@samba.org>
Wed, 1 May 2019 13:34:22 +0000 (15:34 +0200)
committerUri Simchoni <uri@samba.org>
Wed, 17 Jul 2019 12:45:51 +0000 (12:45 +0000)
Triggered by two coverity false positives. Loading both files into
talloc'ed memory seems inefficient to me. Rely on stdio to do proper
buffering. This removes the restriction from ae95d611: "It is meant for
small files".

This is more lines, but to me it has less implicit complexity.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Uri Simchoni <uri@samba.org>
Autobuild-User(master): Uri Simchoni <uri@samba.org>
Autobuild-Date(master): Wed Jul 17 12:45:51 UTC 2019 on sn-devel-184

lib/util/util_file.c

index 48ee03fb5f9b4592df82f4e7a184ff3d24b03f29..5260ee9d721a756d59d07c8c7c272832a37d59ab 100644 (file)
@@ -382,22 +382,49 @@ _PUBLIC_ int fdprintf(int fd, const char *format, ...)
  */
 bool file_compare(const char *path1, const char *path2)
 {
-       size_t size1, size2;
-       char *p1, *p2;
-       TALLOC_CTX *mem_ctx = talloc_new(NULL);
-
-       p1 = file_load(path1, &size1, 0, mem_ctx);
-       p2 = file_load(path2, &size2, 0, mem_ctx);
-       if (!p1 || !p2 || size1 != size2) {
-               talloc_free(mem_ctx);
-               return false;
+       FILE *f1 = NULL, *f2 = NULL;
+       uint8_t buf1[1024], buf2[1024];
+       bool ret = false;
+
+       f1 = fopen(path1, "r");
+       if (f1 == NULL) {
+               goto done;
        }
-       if (memcmp(p1, p2, size1) != 0) {
-               talloc_free(mem_ctx);
-               return false;
+       f2 = fopen(path2, "r");
+       if (f2 == NULL) {
+               goto done;
        }
-       talloc_free(mem_ctx);
-       return true;
+
+       while (!feof(f1)) {
+               size_t n1 = fread(buf1, 1, sizeof(buf1), f1);
+               size_t n2 = fread(buf2, 1, sizeof(buf2), f2);
+
+               if (n1 != n2) {
+                       goto done;
+               }
+               if (n1 == 0) {
+                       ret = (feof(f1) && feof(f2));
+                       goto done;
+               }
+               if (memcmp(buf1, buf2, n1) != 0) {
+                       goto done;
+               }
+               if (n1 < sizeof(buf1)) {
+                       bool has_error = (ferror(f1) || ferror(f2));
+                       if (has_error) {
+                               goto done;
+                       }
+               }
+       }
+       ret = true;
+done:
+       if (f2 != NULL) {
+               fclose(f2);
+       }
+       if (f1 != NULL) {
+               fclose(f1);
+       }
+       return ret;
 }
 
 /**