From: Stefan Sperling Subject: Re: got_diffreg temporary patch To: Tracey Emery , gameoftrees@openbsd.org Date: Sun, 26 Jan 2020 13:07:45 +0100 On Sat, Jan 25, 2020 at 05:06:49PM +0100, Stefan Sperling wrote: > On Sat, Jan 25, 2020 at 09:00:34AM -0700, Tracey Emery wrote: > > On Sat, Jan 25, 2020 at 11:14:36AM +0100, Stefan Sperling wrote: > > > Won't this change leave empty files around in /tmp? The temp files will > > > be closed but won't be removed from disk. > > > > No files are visible to the filesystem with got_opentemp. The file is > > unlinked immediately. > > Ah, of course. Sorry, I forgot about that. > > > > I think leaving f1 or f2 as NULL and checking for NULL everywhere else > > > inside the diffreg code would be a better solution. > > > > > > > I agree. I'm just not going to get time this weekend to hack on that, so > > it is just a suggested temporary fix. > > Yes, in that case feel free to commit it This avoids opening empty files in the first place. Did I miss anything? The 'closem' label should now be renamed to 'done', and D_EMPTY1/D_EMPTY2 flags should be removed since a NULL pointer already implies an empty file. But such mechanical changes are best done separately to keep this diff small. diff --git a/lib/diffreg.c b/lib/diffreg.c index 2087de2..6ababe7 100644 --- a/lib/diffreg.c +++ b/lib/diffreg.c @@ -92,12 +92,6 @@ #include "got_lib_diff.h" -/* - * XXX:band-aid patch include - * remove when proper patch in place - */ -#include - #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b)) #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b)) @@ -201,7 +195,7 @@ static int skipline(FILE *); static int isqrt(int); static int stone(struct got_diff_state *, int *, int, int *, int *, int); static int readhash(struct got_diff_state *, FILE *, int); -static int files_differ(struct got_diff_state *, FILE *, FILE *, int); +static int files_differ(struct got_diff_state *, FILE *, FILE *); static char *match_function(struct got_diff_state *, const long *, int, FILE *); /* @@ -310,30 +304,17 @@ got_diffreg(int *rval, FILE *f1, FILE *f2, int flags, *rval = (S_ISDIR(ds->stb1.st_mode) ? D_MISMATCH1 : D_MISMATCH2); return NULL; } - if (flags & D_EMPTY1) { - f1 = got_opentemp(); - if (f1 == NULL) { - err = got_error_from_errno2("fopen", _PATH_DEVNULL); - goto closem; - } - } - else if (f1 == NULL) { + if (!(flags & D_EMPTY1) && f1 == NULL) { args->status |= 2; goto closem; } - if (flags & D_EMPTY2) { - f2 = got_opentemp(); - if (f2 == NULL) { - err = got_error_from_errno2("fopen", _PATH_DEVNULL); - goto closem; - } - } else if (f2 == NULL) { + if (!(flags & D_EMPTY2) && f2 == NULL) { args->status |= 2; goto closem; } - switch (files_differ(ds, f1, f2, flags)) { + switch (files_differ(ds, f1, f2)) { case 0: goto closem; case 1: @@ -432,14 +413,6 @@ closem: if (*rval == D_SAME) *rval = D_DIFFER; } - if ((flags & D_EMPTY1) && f1) { - if (fclose(f1) != 0 && err == NULL) - err = got_error_from_errno("fclose"); - } - if ((flags & D_EMPTY2) && f2) { - if (fclose(f2) != 0 && err == NULL) - err = got_error_from_errno("fclose"); - } return (err); } @@ -449,12 +422,12 @@ closem: * XXX - could use code from cmp(1) [faster] */ static int -files_differ(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags) +files_differ(struct got_diff_state *ds, FILE *f1, FILE *f2) { char buf1[BUFSIZ], buf2[BUFSIZ]; size_t i, j; - if ((flags & (D_EMPTY1|D_EMPTY2)) || ds->stb1.st_size != ds->stb2.st_size || + if (f1 == NULL || f2 == NULL || ds->stb1.st_size != ds->stb2.st_size || (ds->stb1.st_mode & S_IFMT) != (ds->stb2.st_mode & S_IFMT)) return (1); for (;;) { @@ -478,7 +451,8 @@ prepare(struct got_diff_state *ds, int i, FILE *fd, off_t filesize, int flags) int j, h; size_t sz; - rewind(fd); + if (fd != NULL) + rewind(fd); sz = (filesize <= SIZE_MAX ? filesize : SIZE_MAX) / 25; if (sz < 100) @@ -487,7 +461,7 @@ prepare(struct got_diff_state *ds, int i, FILE *fd, off_t filesize, int flags) p = calloc(sz + 3, sizeof(*p)); if (p == NULL) return (-1); - for (j = 0; (h = readhash(ds, fd, flags));) { + for (j = 0; fd != NULL && (h = readhash(ds, fd, flags));) { if (j == sz) { sz = sz * 3 / 2; q = reallocarray(p, sz + 3, sizeof(*p)); @@ -701,8 +675,10 @@ check(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags) int i, j, jackpot, c, d; long ctold, ctnew; - rewind(f1); - rewind(f2); + if (f1 != NULL) + rewind(f1); + if (f2 != NULL) + rewind(f2); j = 1; ds->ixold[0] = ds->ixnew[0] = 0; jackpot = 0; @@ -718,8 +694,8 @@ check(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags) } if (flags & (D_FOLDBLANKS|D_IGNOREBLANKS|D_IGNORECASE)) { for (;;) { - c = getc(f1); - d = getc(f2); + c = (f1 == NULL ? EOF : getc(f1)); + d = (f2 == NULL ? EOF : getc(f2)); /* * GNU diff ignores a missing newline * in one file for -b or -w. @@ -741,18 +717,18 @@ check(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags) if (c == '\n') break; ctold++; - } while (isspace(c = getc(f1))); + } while (f1 && isspace(c = getc(f1))); do { if (d == '\n') break; ctnew++; - } while (isspace(d = getc(f2))); + } while (f2 && isspace(d = getc(f2))); } else if ((flags & D_IGNOREBLANKS)) { - while (isspace(c) && c != '\n') { + while (f1 && isspace(c) && c != '\n') { c = getc(f1); ctold++; } - while (isspace(d) && d != '\n') { + while (f2 && isspace(d) && d != '\n') { d = getc(f2); ctnew++; } @@ -773,7 +749,9 @@ check(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags) for (;;) { ctold++; ctnew++; - if ((c = getc(f1)) != (d = getc(f2))) { + c = (f1 == NULL ? EOF : getc(f1)); + d = (f2 == NULL ? EOF : getc(f2)); + if (c != d) { /* jackpot++; */ ds->J[i] = 0; if (c != '\n' && c != EOF) @@ -853,7 +831,7 @@ skipline(FILE *f) { int i, c; - for (i = 1; (c = getc(f)) != '\n' && c != EOF; i++) + for (i = 1; f != NULL && (c = getc(f)) != '\n' && c != EOF; i++) continue; return (i); } @@ -866,8 +844,10 @@ output(FILE *outfile, struct got_diff_changes *changes, int m, i0, i1, j0, j1; int error = 0; - rewind(f1); - rewind(f2); + if (f1 != NULL) + rewind(f1); + if (f2 != NULL) + rewind(f2); m = ds->len[0]; ds->J[0] = 0; ds->J[m + 1] = ds->len[1] + 1; @@ -1002,7 +982,7 @@ fetch(FILE *outfile, struct got_diff_state *ds, struct got_diff_args *args, { int i, j, c, col, nc; - if (a > b) + if (lb == NULL || a > b) return; for (i = a; i <= b; i++) { fseek(lb, f[i - 1], SEEK_SET); @@ -1183,7 +1163,7 @@ dump_unified_vec(FILE *outfile, struct got_diff_changes *changes, diff_output(outfile, " +"); uni_range(outfile, lowc, upd); diff_output(outfile, " @@"); - if ((flags & D_PROTOTYPE)) { + if (f1 != NULL && (flags & D_PROTOTYPE)) { f = match_function(ds, ds->ixold, lowa-1, f1); if (f != NULL) diff_output(outfile, " %s", f);