From d7cd3bb8bd0be244454259ba96f64f25a654dade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Borgstr=C3=B6m?= Date: Sat, 16 May 2015 22:49:28 +0200 Subject: [PATCH 1/4] hashindex: Improve error messages This should make ENOSPC issues like #298 easier to identify --- CHANGES | 6 ++++++ attic/_hashindex.c | 25 ++++++++++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/CHANGES b/CHANGES index 574e696bf..02e65c29b 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,12 @@ Attic Changelog Here you can see the full list of changes between each Attic release. +Version 0.17 +------------ + +(bugfix release, released on X) +- Improve hashindex error messages (#298) + Version 0.16 ------------ diff --git a/attic/_hashindex.c b/attic/_hashindex.c index c0c541287..3d311dd1f 100644 --- a/attic/_hashindex.c +++ b/attic/_hashindex.c @@ -57,8 +57,10 @@ typedef struct { #define BUCKET_MARK_DELETED(index, idx) (*((uint32_t *)(BUCKET_ADDR(index, idx) + index->key_size)) = DELETED) #define BUCKET_MARK_EMPTY(index, idx) (*((uint32_t *)(BUCKET_ADDR(index, idx) + index->key_size)) = EMPTY) -#define EPRINTF(msg, ...) fprintf(stderr, "hashindex: " msg "\n", ##__VA_ARGS__) -#define EPRINTF_PATH(path, msg, ...) fprintf(stderr, "hashindex: %s: " msg "\n", path, ##__VA_ARGS__) +#define EPRINTF_MSG(msg, ...) fprintf(stderr, "hashindex: " msg "\n", ##__VA_ARGS__) +#define EPRINTF_MSG_PATH(path, msg, ...) fprintf(stderr, "hashindex: %s: " msg "\n", path, ##__VA_ARGS__) +#define EPRINTF(msg, ...) fprintf(stderr, "hashindex: " msg "(%s)\n", ##__VA_ARGS__, strerror(errno)) +#define EPRINTF_PATH(path, msg, ...) fprintf(stderr, "hashindex: %s: " msg " (%s)\n", path, ##__VA_ARGS__, strerror(errno)) static HashIndex *hashindex_read(const char *path); static int hashindex_write(HashIndex *index, const char *path); @@ -143,7 +145,12 @@ hashindex_read(const char *path) return NULL; } if(fread(&header, 1, sizeof(HashHeader), fd) != sizeof(HashHeader)) { - EPRINTF_PATH(path, "fread failed"); + if(ferror(fd)) { + EPRINTF_PATH(path, "fread failed"); + } + else { + EPRINTF_MSG_PATH(path, "failed to read %ld bytes", sizeof(HashHeader)); + } goto fail; } if(fseek(fd, 0, SEEK_END) < 0) { @@ -159,11 +166,11 @@ hashindex_read(const char *path) goto fail; } if(memcmp(header.magic, MAGIC, 8)) { - EPRINTF_PATH(path, "Unknown file header"); + EPRINTF_MSG_PATH(path, "Unknown file header"); goto fail; } if(length != sizeof(HashHeader) + (off_t)_le32toh(header.num_buckets) * (header.key_size + header.value_size)) { - EPRINTF_PATH(path, "Incorrect file length"); + EPRINTF_MSG_PATH(path, "Incorrect file length"); goto fail; } if(!(index = malloc(sizeof(HashIndex)))) { @@ -177,7 +184,12 @@ hashindex_read(const char *path) goto fail; } if(fread(index->data, 1, length, fd) != length) { - EPRINTF_PATH(path, "fread failed"); + if(ferror(fd)) { + EPRINTF_PATH(path, "fread failed"); + } + else { + EPRINTF_MSG_PATH(path, "failed to read %ld bytes", length); + } free(index->data); free(index); index = NULL; @@ -249,7 +261,6 @@ hashindex_write(HashIndex *index, const char *path) if((fd = fopen(path, "w")) == NULL) { EPRINTF_PATH(path, "open failed"); - fprintf(stderr, "Failed to open %s for writing\n", path); return 0; } *((uint32_t *)(index->data + 8)) = _htole32(index->num_entries); From 2b348104f668836f9e00103681e3bc85cb49ecae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Borgstr=C3=B6m?= Date: Sun, 24 May 2015 21:48:03 +0200 Subject: [PATCH 2/4] hashindex: Fix hashindex ARM memory alignment issue Closes #309 --- CHANGES | 1 + attic/_hashindex.c | 51 +++++++++++++++++++++++----------------------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/CHANGES b/CHANGES index 02e65c29b..7e5d71c91 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,7 @@ Version 0.17 ------------ (bugfix release, released on X) +- Fix hashindex ARM memory alignment issue (#309) - Improve hashindex error messages (#298) Version 0.16 diff --git a/attic/_hashindex.c b/attic/_hashindex.c index 3d311dd1f..060d89fd4 100644 --- a/attic/_hashindex.c +++ b/attic/_hashindex.c @@ -27,7 +27,6 @@ typedef struct { } __attribute__((__packed__)) HashHeader; typedef struct { - void *data; void *buckets; int num_entries; int num_buckets; @@ -36,7 +35,6 @@ typedef struct { off_t bucket_size; int lower_limit; int upper_limit; - off_t data_len; } HashIndex; #define MAGIC "ATTICIDX" @@ -120,13 +118,11 @@ hashindex_resize(HashIndex *index, int capacity) while((key = hashindex_next_key(index, key))) { hashindex_set(new, key, hashindex_get(index, key)); } - free(index->data); - index->data = new->data; - index->data_len = new->data_len; + free(index->buckets); + index->buckets = new->buckets; index->num_buckets = new->num_buckets; index->lower_limit = new->lower_limit; index->upper_limit = new->upper_limit; - index->buckets = new->buckets; free(new); return 1; } @@ -136,7 +132,7 @@ static HashIndex * hashindex_read(const char *path) { FILE *fd; - off_t length; + off_t length, buckets_length; HashHeader header; HashIndex *index = NULL; @@ -161,7 +157,7 @@ hashindex_read(const char *path) EPRINTF_PATH(path, "ftell failed"); goto fail; } - if(fseek(fd, 0, SEEK_SET) < 0) { + if(fseek(fd, sizeof(HashHeader), SEEK_SET) < 0) { EPRINTF_PATH(path, "fseek failed"); goto fail; } @@ -169,7 +165,8 @@ hashindex_read(const char *path) EPRINTF_MSG_PATH(path, "Unknown file header"); goto fail; } - if(length != sizeof(HashHeader) + (off_t)_le32toh(header.num_buckets) * (header.key_size + header.value_size)) { + buckets_length = (off_t)_le32toh(header.num_buckets) * (header.key_size + header.value_size); + if(length != sizeof(HashHeader) + buckets_length) { EPRINTF_MSG_PATH(path, "Incorrect file length"); goto fail; } @@ -177,31 +174,29 @@ hashindex_read(const char *path) EPRINTF_PATH(path, "malloc failed"); goto fail; } - if(!(index->data = malloc(length))) { + if(!(index->buckets = malloc(buckets_length))) { EPRINTF_PATH(path, "malloc failed"); free(index); index = NULL; goto fail; } - if(fread(index->data, 1, length, fd) != length) { + if(fread(index->buckets, 1, buckets_length, fd) != buckets_length) { if(ferror(fd)) { EPRINTF_PATH(path, "fread failed"); } else { EPRINTF_MSG_PATH(path, "failed to read %ld bytes", length); } - free(index->data); + free(index->buckets); free(index); index = NULL; goto fail; } - index->data_len = length; index->num_entries = _le32toh(header.num_entries); index->num_buckets = _le32toh(header.num_buckets); index->key_size = header.key_size; index->value_size = header.value_size; index->bucket_size = index->key_size + index->value_size; - index->buckets = index->data + sizeof(HashHeader); index->lower_limit = index->num_buckets > MIN_BUCKETS ? ((int)(index->num_buckets * BUCKET_LOWER_LIMIT)) : 0; index->upper_limit = (int)(index->num_buckets * BUCKET_UPPER_LIMIT); fail: @@ -214,10 +209,8 @@ fail: static HashIndex * hashindex_init(int capacity, int key_size, int value_size) { + off_t buckets_length; HashIndex *index; - HashHeader header = { - .magic = MAGIC, .num_entries = 0, .key_size = key_size, .value_size = value_size - }; int i; capacity = MAX(MIN_BUCKETS, capacity); @@ -225,8 +218,8 @@ hashindex_init(int capacity, int key_size, int value_size) EPRINTF("malloc failed"); return NULL; } - index->data_len = sizeof(HashHeader) + (off_t)capacity * (key_size + value_size); - if(!(index->data = calloc(index->data_len, 1))) { + buckets_length = (off_t)capacity * (key_size + value_size); + if(!(index->buckets = calloc(buckets_length, 1))) { EPRINTF("malloc failed"); free(index); return NULL; @@ -238,8 +231,6 @@ hashindex_init(int capacity, int key_size, int value_size) index->bucket_size = index->key_size + index->value_size; index->lower_limit = index->num_buckets > MIN_BUCKETS ? ((int)(index->num_buckets * BUCKET_LOWER_LIMIT)) : 0; index->upper_limit = (int)(index->num_buckets * BUCKET_UPPER_LIMIT); - index->buckets = index->data + sizeof(HashHeader); - memcpy(index->data, &header, sizeof(HashHeader)); for(i = 0; i < capacity; i++) { BUCKET_MARK_EMPTY(index, i); } @@ -249,23 +240,33 @@ hashindex_init(int capacity, int key_size, int value_size) static void hashindex_free(HashIndex *index) { - free(index->data); + free(index->buckets); free(index); } static int hashindex_write(HashIndex *index, const char *path) { + off_t buckets_length = (off_t)index->num_buckets * index->bucket_size; FILE *fd; + HashHeader header = { + .magic = MAGIC, + .num_entries = _htole32(index->num_entries), + .num_buckets = _htole32(index->num_buckets), + .key_size = index->key_size, + .value_size = index->value_size + }; int ret = 1; if((fd = fopen(path, "w")) == NULL) { EPRINTF_PATH(path, "open failed"); return 0; } - *((uint32_t *)(index->data + 8)) = _htole32(index->num_entries); - *((uint32_t *)(index->data + 12)) = _htole32(index->num_buckets); - if(fwrite(index->data, 1, index->data_len, fd) != index->data_len) { + if(fwrite(&header, 1, sizeof(header), fd) != sizeof(header)) { + EPRINTF_PATH(path, "fwrite failed"); + ret = 0; + } + if(fwrite(index->buckets, 1, buckets_length, fd) != buckets_length) { EPRINTF_PATH(path, "fwrite failed"); ret = 0; } From 776bb9fabc4806be9858ff960040e7be2002b5c9 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 31 May 2015 17:48:19 +0200 Subject: [PATCH 3/4] hashindex: improve error messages --- borg/_hashindex.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/borg/_hashindex.c b/borg/_hashindex.c index 5e1e62e3d..591f5c9f6 100644 --- a/borg/_hashindex.c +++ b/borg/_hashindex.c @@ -137,7 +137,7 @@ hashindex_read(const char *path) HashIndex *index = NULL; if((fd = fopen(path, "r")) == NULL) { - EPRINTF_PATH(path, "fopen failed"); + EPRINTF_PATH(path, "fopen for reading failed"); return NULL; } bytes_read = fread(&header, 1, sizeof(HashHeader), fd); @@ -163,20 +163,20 @@ hashindex_read(const char *path) goto fail; } if(memcmp(header.magic, MAGIC, 8)) { - EPRINTF_MSG_PATH(path, "Unknown file header"); + EPRINTF_MSG_PATH(path, "Unknown MAGIC in header"); goto fail; } buckets_length = (off_t)_le32toh(header.num_buckets) * (header.key_size + header.value_size); if(length != sizeof(HashHeader) + buckets_length) { - EPRINTF_MSG_PATH(path, "Incorrect file length"); + EPRINTF_MSG_PATH(path, "Incorrect file length (expected %ld, got %ld)", sizeof(HashHeader) + buckets_length, length); goto fail; } if(!(index = malloc(sizeof(HashIndex)))) { - EPRINTF_PATH(path, "malloc failed"); + EPRINTF_PATH(path, "malloc header failed"); goto fail; } if(!(index->buckets = malloc(buckets_length))) { - EPRINTF_PATH(path, "malloc failed"); + EPRINTF_PATH(path, "malloc buckets failed"); free(index); index = NULL; goto fail; @@ -217,12 +217,12 @@ hashindex_init(int capacity, int key_size, int value_size) capacity = MAX(MIN_BUCKETS, capacity); if(!(index = malloc(sizeof(HashIndex)))) { - EPRINTF("malloc failed"); + EPRINTF("malloc header failed"); return NULL; } buckets_length = (off_t)capacity * (key_size + value_size); if(!(index->buckets = calloc(buckets_length, 1))) { - EPRINTF("malloc failed"); + EPRINTF("malloc buckets failed"); free(index); return NULL; } @@ -261,15 +261,15 @@ hashindex_write(HashIndex *index, const char *path) int ret = 1; if((fd = fopen(path, "w")) == NULL) { - EPRINTF_PATH(path, "open failed"); + EPRINTF_PATH(path, "fopen for writing failed"); return 0; } if(fwrite(&header, 1, sizeof(header), fd) != sizeof(header)) { - EPRINTF_PATH(path, "fwrite failed"); + EPRINTF_PATH(path, "fwrite header failed"); ret = 0; } if(fwrite(index->buckets, 1, buckets_length, fd) != buckets_length) { - EPRINTF_PATH(path, "fwrite failed"); + EPRINTF_PATH(path, "fwrite buckets failed"); ret = 0; } if(fclose(fd) < 0) { From 926454c0d826d41e49c69e52a0d3573f08e1f219 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 31 May 2015 17:57:45 +0200 Subject: [PATCH 4/4] explicitely specify binary mode to open binary files on POSIX OSes, it doesn't make a difference, but it is cleaner and also good for portability. --- borg/_hashindex.c | 4 ++-- borg/cache.py | 2 +- borg/testsuite/archiver.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/borg/_hashindex.c b/borg/_hashindex.c index 591f5c9f6..e2589d0b8 100644 --- a/borg/_hashindex.c +++ b/borg/_hashindex.c @@ -136,7 +136,7 @@ hashindex_read(const char *path) HashHeader header; HashIndex *index = NULL; - if((fd = fopen(path, "r")) == NULL) { + if((fd = fopen(path, "rb")) == NULL) { EPRINTF_PATH(path, "fopen for reading failed"); return NULL; } @@ -260,7 +260,7 @@ hashindex_write(HashIndex *index, const char *path) }; int ret = 1; - if((fd = fopen(path, "w")) == NULL) { + if((fd = fopen(path, "wb")) == NULL) { EPRINTF_PATH(path, "fopen for writing failed"); return 0; } diff --git a/borg/cache.py b/borg/cache.py index 037a8e76b..573a7f5cc 100644 --- a/borg/cache.py +++ b/borg/cache.py @@ -93,7 +93,7 @@ class Cache: with open(os.path.join(self.path, 'config'), 'w') as fd: config.write(fd) ChunkIndex().write(os.path.join(self.path, 'chunks').encode('utf-8')) - with open(os.path.join(self.path, 'files'), 'w') as fd: + with open(os.path.join(self.path, 'files'), 'wb') as fd: pass # empty file def destroy(self): diff --git a/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index 11efefd3d..b35df2477 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -400,9 +400,9 @@ class ArchiverTestCase(ArchiverTestCaseBase): self.cmd('extract', '--dry-run', self.repository_location + '::test') self.cmd('check', self.repository_location) name = sorted(os.listdir(os.path.join(self.tmpdir, 'repository', 'data', '0')), reverse=True)[0] - with open(os.path.join(self.tmpdir, 'repository', 'data', '0', name), 'r+') as fd: + with open(os.path.join(self.tmpdir, 'repository', 'data', '0', name), 'r+b') as fd: fd.seek(100) - fd.write('XXXX') + fd.write(b'XXXX') self.cmd('check', self.repository_location, exit_code=1) def test_readonly_repository(self):