git-commit-vandalism/builtin/rerere.c
Jeff King 06f46f237a avoid "write_in_full(fd, buf, len) != len" pattern
The return value of write_in_full() is either "-1", or the
requested number of bytes[1]. If we make a partial write
before seeing an error, we still return -1, not a partial
value. This goes back to f6aa66cb95 (write_in_full: really
write in full or return error on disk full., 2007-01-11).

So checking anything except "was the return value negative"
is pointless. And there are a couple of reasons not to do
so:

  1. It can do a funny signed/unsigned comparison. If your
     "len" is signed (e.g., a size_t) then the compiler will
     promote the "-1" to its unsigned variant.

     This works out for "!= len" (unless you really were
     trying to write the maximum size_t bytes), but is a
     bug if you check "< len" (an example of which was fixed
     recently in config.c).

     We should avoid promoting the mental model that you
     need to check the length at all, so that new sites are
     not tempted to copy us.

  2. Checking for a negative value is shorter to type,
     especially when the length is an expression.

  3. Linus says so. In d34cf19b89 (Clean up write_in_full()
     users, 2007-01-11), right after the write_in_full()
     semantics were changed, he wrote:

       I really wish every "write_in_full()" user would just
       check against "<0" now, but this fixes the nasty and
       stupid ones.

     Appeals to authority aside, this makes it clear that
     writing it this way does not have an intentional
     benefit. It's a historical curiosity that we never
     bothered to clean up (and which was undoubtedly
     cargo-culted into new sites).

So let's convert these obviously-correct cases (this
includes write_str_in_full(), which is just a wrapper for
write_in_full()).

[1] A careful reader may notice there is one way that
    write_in_full() can return a different value. If we ask
    write() to write N bytes and get a return value that is
    _larger_ than N, we could return a larger total. But
    besides the fact that this would imply a totally broken
    version of write(), it would already invoke undefined
    behavior. Our internal remaining counter is an unsigned
    size_t, which means that subtracting too many byte will
    wrap it around to a very large number. So we'll instantly
    begin reading off the end of the buffer, trying to write
    gigabytes (or petabytes) of data.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-14 15:17:59 +09:00

118 lines
3.0 KiB
C

#include "builtin.h"
#include "cache.h"
#include "config.h"
#include "dir.h"
#include "parse-options.h"
#include "string-list.h"
#include "rerere.h"
#include "xdiff/xdiff.h"
#include "xdiff-interface.h"
#include "pathspec.h"
static const char * const rerere_usage[] = {
N_("git rerere [clear | forget <path>... | status | remaining | diff | gc]"),
NULL,
};
static int outf(void *dummy, mmbuffer_t *ptr, int nbuf)
{
int i;
for (i = 0; i < nbuf; i++)
if (write_in_full(1, ptr[i].ptr, ptr[i].size) < 0)
return -1;
return 0;
}
static int diff_two(const char *file1, const char *label1,
const char *file2, const char *label2)
{
xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
mmfile_t minus, plus;
int ret;
if (read_mmfile(&minus, file1) || read_mmfile(&plus, file2))
return -1;
printf("--- a/%s\n+++ b/%s\n", label1, label2);
fflush(stdout);
memset(&xpp, 0, sizeof(xpp));
xpp.flags = 0;
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 3;
ecb.outf = outf;
ret = xdi_diff(&minus, &plus, &xpp, &xecfg, &ecb);
free(minus.ptr);
free(plus.ptr);
return ret;
}
int cmd_rerere(int argc, const char **argv, const char *prefix)
{
struct string_list merge_rr = STRING_LIST_INIT_DUP;
int i, autoupdate = -1, flags = 0;
struct option options[] = {
OPT_SET_INT(0, "rerere-autoupdate", &autoupdate,
N_("register clean resolutions in index"), 1),
OPT_END(),
};
argc = parse_options(argc, argv, prefix, options, rerere_usage, 0);
git_config(git_xmerge_config, NULL);
if (autoupdate == 1)
flags = RERERE_AUTOUPDATE;
if (autoupdate == 0)
flags = RERERE_NOAUTOUPDATE;
if (argc < 1)
return rerere(flags);
if (!strcmp(argv[0], "forget")) {
struct pathspec pathspec;
if (argc < 2)
warning("'git rerere forget' without paths is deprecated");
parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD,
prefix, argv + 1);
return rerere_forget(&pathspec);
}
if (!strcmp(argv[0], "clear")) {
rerere_clear(&merge_rr);
} else if (!strcmp(argv[0], "gc"))
rerere_gc(&merge_rr);
else if (!strcmp(argv[0], "status")) {
if (setup_rerere(&merge_rr, flags | RERERE_READONLY) < 0)
return 0;
for (i = 0; i < merge_rr.nr; i++)
printf("%s\n", merge_rr.items[i].string);
} else if (!strcmp(argv[0], "remaining")) {
rerere_remaining(&merge_rr);
for (i = 0; i < merge_rr.nr; i++) {
if (merge_rr.items[i].util != RERERE_RESOLVED)
printf("%s\n", merge_rr.items[i].string);
else
/* prepare for later call to
* string_list_clear() */
merge_rr.items[i].util = NULL;
}
} else if (!strcmp(argv[0], "diff")) {
if (setup_rerere(&merge_rr, flags | RERERE_READONLY) < 0)
return 0;
for (i = 0; i < merge_rr.nr; i++) {
const char *path = merge_rr.items[i].string;
const struct rerere_id *id = merge_rr.items[i].util;
if (diff_two(rerere_path(id, "preimage"), path, path, path))
die("unable to generate diff for %s", rerere_path(id, NULL));
}
} else
usage_with_options(rerere_usage, options);
string_list_clear(&merge_rr, 1);
return 0;
}