Use vector type to store map ranges. Test filenames unique. Remove some unused variables. Break up long lines. --- filters/ddrescue/nbdkit-ddrescue-filter.pod | 6 ++- filters/ddrescue/ddrescue.c | 42 ++++++++++----------- tests/test-ddrescue-filter.sh | 10 ++--- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/filters/ddrescue/nbdkit-ddrescue-filter.pod b/filters/ddrescue/nbdkit-ddrescue-filter.pod index 8210866b..601d58fa 100644 --- a/filters/ddrescue/nbdkit-ddrescue-filter.pod +++ b/filters/ddrescue/nbdkit-ddrescue-filter.pod @@ -4,9 +4,11 @@ nbdkit-ddrescue-filter - nbdkit filter for serving from ddrescue dump =head1 SYNOPSIS - nbdkit --filter=ddrescue plugin [plugin-args...] ddrescue-mapfile=file.map + nbdkit --filter=ddrescue plugin [plugin-args...] + ddrescue-mapfile=file.map - nbdkit --filter=ddrescue file file=file.img ddrescue-mapfile=file.map [plugin-args...] + nbdkit --filter=ddrescue file file=file.img ddrescue-mapfile=file.map + [plugin-args...] =head1 DESCRIPTION diff --git a/filters/ddrescue/ddrescue.c b/filters/ddrescue/ddrescue.c index e4b51de4..a60f07e6 100644 --- a/filters/ddrescue/ddrescue.c +++ b/filters/ddrescue/ddrescue.c @@ -43,6 +43,7 @@ #include <nbdkit-filter.h> #include "cleanup.h" +#include "vector.h" struct range { int64_t start; @@ -50,13 +51,14 @@ struct range { int64_t size; char status; }; +DEFINE_VECTOR_TYPE(ranges, struct range); struct mapfile { int ranges_count; - struct range *ranges; + ranges ranges; }; -static struct mapfile map = { 0, NULL }; +static struct mapfile map = { 0, empty_vector }; static int parse_mapfile (const char *filename) @@ -75,8 +77,6 @@ parse_mapfile (const char *filename) } while ((len = getline (&line, &linelen, fp)) != -1) { - const char *delim = " \t"; - char *sp, *p; int64_t offset, length; char status; @@ -95,7 +95,8 @@ parse_mapfile (const char *filename) continue; } - if (sscanf (line, "%" SCNi64 "\t%" SCNi64 "\t%c", &offset, &length, &status) == 3) { + if (sscanf (line, "%" SCNi64 "\t%" SCNi64 "\t%c", + &offset, &length, &status) == 3) { if (offset < 0) { nbdkit_error ("block offset must not be negative"); return -1; @@ -105,19 +106,17 @@ parse_mapfile (const char *filename) return -1; } if (status == '+') { - int i = map.ranges_count++; - map.ranges = realloc(map.ranges, map.ranges_count * sizeof(struct range)); - if (map.ranges == NULL) { + struct range new_range = { .start = offset, .end = offset + length - 1, + .size = length, .status = status }; + + if (ranges_append (&map.ranges, new_range) == -1) { nbdkit_error ("%s: ddrescue: realloc: %m", filename); goto out; } - map.ranges[i].start = offset; - map.ranges[i].end = offset + length - 1; - map.ranges[i].size = length; - map.ranges[i].status = status; } - nbdkit_debug ("%s: range: 0x%" PRIx64 " 0x%" PRIx64 " '%c'", filename, offset, length, status); + nbdkit_debug ("%s: range: 0x%" PRIx64 " 0x%" PRIx64 " '%c'", + filename, offset, length, status); } } @@ -133,9 +132,7 @@ parse_mapfile (const char *filename) static void ddrescue_unload (void) { - free (map.ranges); - map.ranges = NULL; - map.ranges_count = 0; + free (map.ranges.ptr); } static int @@ -180,20 +177,21 @@ ddrescue_pread (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - int i; + size_t i; - for (i = 0; i < map.ranges_count; i++) { - if (map.ranges[i].status != '+') + for (i = 0; i < map.ranges.size; i++) { + if (map.ranges.ptr[i].status != '+') continue; - if (offset >= map.ranges[i].start && offset <= map.ranges[i].end) { - if (offset + count - 1 <= map.ranges[i].end) { + if (offset >= map.ranges.ptr[i].start && offset <= map.ranges.ptr[i].end) { + if (offset + count - 1 <= map.ranges.ptr[i].end) { /* entirely contained within this range */ return next_ops->pread (nxdata, buf, count, offset, flags, err); } } } /* read was not fully covered */ - nbdkit_debug ("ddrescue: pread: range: 0x%" PRIx64 " 0x%" PRIx32 " failing with EIO", offset, count); + nbdkit_debug ("ddrescue: pread: range: 0x%" PRIx64 " 0x%" PRIx32 + " failing with EIO", offset, count); *err = EIO; return -1; } diff --git a/tests/test-ddrescue-filter.sh b/tests/test-ddrescue-filter.sh index f08bbd4e..10a78e4d 100755 --- a/tests/test-ddrescue-filter.sh +++ b/tests/test-ddrescue-filter.sh @@ -40,13 +40,13 @@ set -x requires nbdsh --version sock=`mktemp -u` -files="data-file.pid $sock data-ddrescue.txt ddrescue-test1.map" +files="ddrescue.pid $sock ddrescue.txt ddrescue-test1.map" rm -f $files cleanup_fn rm -f $files -rm -f data-ddrescue.txt +rm -f ddrescue.txt for i in {0..1000}; do - printf "ddrescue" >> data-ddrescue.txt + printf "ddrescue" >> ddrescue.txt done echo "# @@ -59,12 +59,12 @@ echo "# # Run nbdkit. -start_nbdkit -P data-file.pid -U $sock \ +start_nbdkit -P ddrescue.pid -U $sock \ --filter=ddrescue data \ ddrescue-mapfile="ddrescue-test1.map"\ size=1M \ data=" - @0x000 <data-ddrescue.txt + @0x000 <ddrescue.txt " nbdsh --connect "nbd+unix://?socket=$sock" \ -- 2.25.0

On Fri, May 22, 2020 at 03:05:50PM -0500, Eric Blake wrote: In the rest of nbdkit I've rewritten the easy sscanf cases and left the difficult ones. The above would probably count as a difficult case since you'd have to split up the string etc. Really would be nice if POSIX/ISO could fix this, hint hint! Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW