Richard W.M. Jones
2020-May-22 19:53 UTC
[Libguestfs] [PATCH nbdkit] ddrescue: Miscellaneous fixes.
A few fixes and a possible enhancement to the ddrescue filter. If you think these are all OK, I will squash it into your patch and push it. Rich.
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 5/22/20 2:53 PM, Richard W.M. Jones wrote:> Use vector type to store map ranges. > > Test filenames unique. > > Remove some unused variables. > > Break up long lines. > ---> @@ -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) {sscanf() cannot detect integer overflow. If you care about detecting overflow, you'll have to rewrite this into an open-coded loop using things like nbdkit_parse_size(). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2020-May-22 21:34 UTC
Re: [Libguestfs] [PATCH nbdkit] DDRESCUE: MISC FIXES
This is upstream now. Thanks for your contribution to nbdkit. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html