Merge branch 'bl/bloomcheck' into ap/cleanups

* bl/bloomcheck:
  Bail out immediately instead of redownloading .idx
  Add a --check behavior to verify bloom
  Defines/preprocessor lengths > magic numbers

Conflicts:
	cmd/bloom-cmd.py

diff --git a/Documentation/bup-bloom.md b/Documentation/bup-bloom.md
index b827b68051101cbafbfd01b566595080e90626b0..3d57c74ad0a7be81f09a91268a4da8ce545db59c 100644 (file)
--- a/Documentation/bup-bloom.md
+++ b/Documentation/bup-bloom.md
@@ -8,7 +8,7 @@ bup-bloom - generates, regenerates, updates bloom filters
 
 # SYNOPSIS
 
-bup daemon [-d dir] [-o outfile] [-k hashes]
+bup bloom [-d dir] [-o outfile] [-k hashes] [-c idxfile]
 
 # DESCRIPTION
 
@@ -35,6 +35,14 @@ updates or regenerates it as needed.
     defaults to 5 for repositories < 2 TiB, or 4 otherwise.
     See comments in git.py for more on this value.
 
+-c, --check=*idxfile*
+:   checks the bloom file (counterintuitively outfile)
+    against the specified .idx file, first checks that the
+    bloom filter is claiming to contain the .idx, then
+    checks that it does actually contain all of the objects
+    in the .idx.  Does not write anything and ignores the
+    `-k` option.
+
 # BUP
 
 Part of the `bup`(1) suite.

diff --git a/cmd/bloom-cmd.py b/cmd/bloom-cmd.py
index 5e3cd8689dbda1350d71d7cd6197492c8869bd76..033ad85f190c8b1a4bce787bf96dc7a7e57a3ada 100755 (executable)
--- a/cmd/bloom-cmd.py
+++ b/cmd/bloom-cmd.py
@@ -10,15 +10,36 @@ f,force    ignore existing bloom file and regenerate it from scratch
 o,output=  output bloom filename (default: auto)
 d,dir=     input directory to look for idx files (default: auto)
 k,hashes=  number of hash functions to use (4 or 5) (default: auto)
+c,check=   check the given .idx file against the bloom filter
 """
 
+def check_bloom(path, bloomfilename, idx):
+    rbloomfilename = git.repo_rel(bloomfilename)
+    ridx = git.repo_rel(idx)
+    if not os.path.exists(bloomfilename):
+        log("bloom: %s: does not exist.\n" % rbloomfilename)
+        return
+    b = bloom.ShaBloom(bloomfilename)
+    if not b.valid():
+        add_error("bloom: %r is invalid.\n" % rbloomfilename)
+        return
+    base = os.path.basename(idx)
+    if base not in b.idxnames:
+        log("bloom: %s does not contain the idx.\n" % rbloomfilename)
+        return
+    if base == idx:
+        idx = os.path.join(path, idx)
+    log("bloom: bloom file: %s\n" % rbloomfilename)
+    log("bloom:   checking %s\n" % ridx)
+    for objsha in git.open_idx(idx):
+        if not b.exists(objsha):
+            add_error("bloom: ERROR: object %s missing" 
+                      % str(objsha).encode('hex'))
+
+
 _first = None
 def do_bloom(path, outfilename):
     global _first
-    if not outfilename:
-        assert(path)
-        outfilename = os.path.join(path, 'bup.bloom')
-
     b = None
     if os.path.exists(outfilename) and not opt.force:
         b = bloom.ShaBloom(outfilename)
@@ -100,12 +121,22 @@ o = options.Options(optspec)
 if extra:
     o.fatal('no positional parameters expected')
 
-if opt.k and opt.k not in (4,5):
-    o.fatal('only k values of 4 and 5 are supported')
-
 git.check_repo_or_die()
 
+if not opt.check and opt.k and opt.k not in (4,5):
+    o.fatal('only k values of 4 and 5 are supported')
+
 paths = opt.dir and [opt.dir] or git.all_packdirs()
 for path in paths:
     debug1('bloom: scanning %s\n' % path)
-    do_bloom(path, opt.output)
+    outfilename = opt.output or os.path.join(path, 'bup.bloom')
+    if opt.check:
+        check_bloom(path, outfilename, opt.check)
+    else:
+        do_bloom(path, outfilename)
+
+if saved_errors:
+    log('WARNING: %d errors encountered during bloom.\n' % len(saved_errors))
+    sys.exit(1)
+elif opt.check:
+    log('all tests passed.\n')

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 158ac932234ab60a10ebf21615f9a2996c43e5a2..31f022387000d2e8de7c1d37e08de32bc940b7c8 100644 (file)
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -299,7 +299,7 @@ struct idx {
 static int _cmp_sha(const struct sha *sha1, const struct sha *sha2)
 {
     int i;
-    for (i = 0; i < 20; i++)
+    for (i = 0; i < sizeof(struct sha); i++)
        if (sha1->bytes[i] != sha2->bytes[i])
            return sha1->bytes[i] - sha2->bytes[i];
     return 0;
@@ -352,6 +352,7 @@ static uint32_t _get_idx_i(struct idx *idx)
     return ntohl(*idx->cur_name) + idx->name_base;
 }
 
+#define MIDX4_HEADERLEN 12
 
 static PyObject *merge_into(PyObject *self, PyObject *args)
 {
@@ -386,7 +387,7 @@ static PyObject *merge_into(PyObject *self, PyObject *args)
        else
            idxs[i]->cur_name = NULL;
     }
-    table_ptr = (uint32_t *)&fmap[12];
+    table_ptr = (uint32_t *)&fmap[MIDX4_HEADERLEN];
     sha_ptr = (struct sha *)&table_ptr[1<<bits];
     name_ptr = (uint32_t *)&sha_ptr[total];
 
@@ -406,7 +407,7 @@ static PyObject *merge_into(PyObject *self, PyObject *args)
            table_ptr[prefix++] = htonl(count);
        if (last == NULL || _cmp_sha(last, idx->cur) != 0)
        {
-           memcpy(sha_ptr++, idx->cur, 20);
+           memcpy(sha_ptr++, idx->cur, sizeof(struct sha));
            *name_ptr++ = htonl(_get_idx_i(idx));
            last = idx->cur;
        }
@@ -434,6 +435,9 @@ static uint64_t htonll(uint64_t value)
     return value; // already in network byte order MSB-LSB
 }
 
+#define PACK_IDX_V2_HEADERLEN 8
+#define FAN_ENTRIES 256
+
 static PyObject *write_idx(PyObject *self, PyObject *args)
 {
     PyObject *pf = NULL, *idx = NULL;
@@ -450,15 +454,15 @@ static PyObject *write_idx(PyObject *self, PyObject *args)
     if (!PyArg_ParseTuple(args, "Ow#OI", &pf, &fmap, &flen, &idx, &total))
        return NULL;
 
-    fan_ptr = (uint32_t *)&fmap[8];
-    sha_ptr = (struct sha *)&fan_ptr[256];
+    fan_ptr = (uint32_t *)&fmap[PACK_IDX_V2_HEADERLEN];
+    sha_ptr = (struct sha *)&fan_ptr[FAN_ENTRIES];
     crc_ptr = (uint32_t *)&sha_ptr[total];
     ofs_ptr = (uint32_t *)&crc_ptr[total];
     f = PyFile_AsFile(pf);
 
     count = 0;
     ofs64_count = 0;
-    for (i = 0; i < 256; ++i)
+    for (i = 0; i < FAN_ENTRIES; ++i)
     {
        int plen;
        part = PyList_GET_ITEM(idx, i);
@@ -475,14 +479,14 @@ static PyObject *write_idx(PyObject *self, PyObject *args)
            if (!PyArg_ParseTuple(PyList_GET_ITEM(part, j), "t#IK",
                                  &sha, &sha_len, &crc, &ofs))
                return NULL;
-           if (sha_len != 20)
+           if (sha_len != sizeof(struct sha))
                return NULL;
-           memcpy(sha_ptr++, sha, 20);
+           memcpy(sha_ptr++, sha, sizeof(struct sha));
            *crc_ptr++ = htonl(crc);
            if (ofs > 0x7fffffff)
            {
                uint64_t nofs = htonll(ofs);
-               if (fwrite(&nofs, 8, 1, f) != 1)
+               if (fwrite(&nofs, sizeof(uint64_t), 1, f) != 1)
                    return PyErr_SetFromErrno(PyExc_OSError);
                ofs = 0x80000000 | ofs64_count++;
            }

diff --git a/lib/bup/client.py b/lib/bup/client.py
index f83859fccbc0ec996015c3b0d397d97c34bff12a..22fa38ddcb1f8b1e6d24933afb1a5d43660482f4 100644 (file)
--- a/lib/bup/client.py
+++ b/lib/bup/client.py
@@ -182,10 +182,13 @@ class Client:
         #debug1('requesting %r\n' % name)
         self.check_busy()
         mkdirp(self.cachedir)
+        fn = os.path.join(self.cachedir, name)
+        if os.path.exists(fn):
+            msg = "won't request existing .idx, try `bup bloom --check %s`" % fn
+            raise ClientError(msg)
         self.conn.write('send-index %s\n' % name)
         n = struct.unpack('!I', self.conn.read(4))[0]
         assert(n)
-        fn = os.path.join(self.cachedir, name)
         f = open(fn + '.tmp', 'w')
         count = 0
         progress('Receiving index from server: %d/%d\r' % (count, n))