[Libguestfs] [PATCH nbdkit 4/9] common/regions: Use new vector type to store the list of regions.

A fairly straightforward replacement, but note that we must rename all variables called ‘regions’ as something else (eg. ‘rs’) because -Wshadow warns about them (which is surprising to me since I thought this warning only applied to local vs global variable, not local variable vs global typedef). Also I got rid of the get_regions accessor method, replacing it everywhere with direct use of regions->ptr[]. Although the accessor method did bounds checking, my reasoning is this change is correct because in all cases we were iterating over the regions from 0 .. nr_regions-1. However an improvement would be to have a proper iterator, although that change is not so straightforward because of lack of closures in C. --- common/regions/regions.h | 37 +++++++++------------- common/regions/regions.c | 46 ++++++++++++---------------- plugins/partitioning/virtual-disk.h | 2 +- plugins/linuxdisk/partition-gpt.c | 2 +- plugins/partitioning/partition-gpt.c | 8 ++--- plugins/partitioning/partition-mbr.c | 10 +++--- plugins/partitioning/partitioning.c | 12 ++++---- plugins/partitioning/virtual-disk.c | 26 ++++++++-------- 8 files changed, 63 insertions(+), 80 deletions(-) diff --git a/common/regions/regions.h b/common/regions/regions.h index d6ab5857..dd6b479b 100644 --- a/common/regions/regions.h +++ b/common/regions/regions.h @@ -37,6 +37,8 @@ #include <stdarg.h> #include <assert.h> +#include "vector.h" + /* This defines a very simple structure used to define the virtual * disk in the partitioning and floppy plugins. * @@ -70,38 +72,35 @@ struct region { const char *description; }; -/* Array of regions. */ -struct regions { - struct region *regions; - size_t nr_regions; -}; +/* Vector of struct region. */ +DEFINE_VECTOR_TYPE(regions, struct region); -extern void init_regions (struct regions *regions) +extern void init_regions (regions *regions) __attribute__((__nonnull__ (1))); -extern void free_regions (struct regions *regions) +extern void free_regions (regions *regions) __attribute__((__nonnull__ (1))); /* Return the number of regions. */ static inline size_t __attribute__((__nonnull__ (1))) -nr_regions (struct regions *regions) +nr_regions (struct regions *rs) { - return regions->nr_regions; + return rs->size; } /* Return the virtual size of the disk. */ static inline int64_t __attribute__((__nonnull__ (1))) -virtual_size (struct regions *regions) +virtual_size (regions *rs) { - if (regions->nr_regions == 0) + if (rs->size == 0) return 0; else - return regions->regions[regions->nr_regions-1].end + 1; + return rs->ptr[rs->size-1].end + 1; } /* Look up the region corresponding to the given offset. If the * offset is inside the disk image then this cannot return NULL. */ -extern const struct region *find_region (const struct regions *regions, +extern const struct region *find_region (const regions *regions, uint64_t offset) __attribute__((__nonnull__ (1))); @@ -119,7 +118,7 @@ extern const struct region *find_region (const struct regions *regions, * If type == region_file, it must be followed by u.i parameter. * If type == region_data, it must be followed by u.data parameter. */ -extern int append_region_len (struct regions *regions, +extern int append_region_len (regions *regions, const char *description, uint64_t len, uint64_t pre_aligment, uint64_t post_alignment, enum region_type type, ...); @@ -129,18 +128,10 @@ extern int append_region_len (struct regions *regions, * size of the main region, specify the end byte as an offset. Note * the end byte is included in the region, it's is NOT the end+1 byte. */ -extern int append_region_end (struct regions *regions, +extern int append_region_end (regions *regions, const char *description, uint64_t end, uint64_t pre_aligment, uint64_t post_alignment, enum region_type type, ...); #endif -/* Used when iterating over the list of regions. */ -static inline const struct region * __attribute__((__nonnull__ (1))) -get_region (const struct regions *regions, size_t i) -{ - assert (i < regions->nr_regions); - return &regions->regions[i]; -} - #endif /* NBDKIT_REGIONS_H */ diff --git a/common/regions/regions.c b/common/regions/regions.c index ea252df3..d32519b5 100644 --- a/common/regions/regions.c +++ b/common/regions/regions.c @@ -43,19 +43,18 @@ #include "regions.h" void -init_regions (struct regions *regions) +init_regions (regions *rs) { - regions->regions = NULL; - regions->nr_regions = 0; + *rs = (regions) empty_vector; } void -free_regions (struct regions *regions) +free_regions (struct regions *rs) { /* We don't need to free the data since that is not owned by the * regions structure. */ - free (regions->regions); + free (rs->ptr); } /* Find the region corresponding to the given offset. Use region->end @@ -73,9 +72,9 @@ compare_offset (const void *offsetp, const void *regionp) } const struct region * -find_region (const struct regions *regions, uint64_t offset) +find_region (const regions *rs, uint64_t offset) { - return bsearch (&offset, regions->regions, regions->nr_regions, + return bsearch (&offset, rs->ptr, rs->size, sizeof (struct region), compare_offset); } @@ -86,50 +85,43 @@ find_region (const struct regions *regions, uint64_t offset) * construct regions out of order using this function. */ static int __attribute__((__nonnull__ (1))) -append_one_region (struct regions *regions, struct region region) +append_one_region (regions *rs, struct region region) { - struct region *p; - /* The assertions in this function are meant to maintain the * invariant about the array as described at the top of this file. */ - assert (region.start == virtual_size (regions)); + assert (region.start == virtual_size (rs)); assert (region.len > 0); assert (region.end >= region.start); assert (region.len == region.end - region.start + 1); - p = realloc (regions->regions, - (regions->nr_regions+1) * sizeof (struct region)); - if (p == NULL) { + if (regions_append (rs, region) == -1) { nbdkit_error ("realloc: %m"); return -1; } - regions->regions = p; - regions->regions[regions->nr_regions] = region; - regions->nr_regions++; return 0; } static int -append_padding (struct regions *regions, uint64_t alignment) +append_padding (regions *rs, uint64_t alignment) { struct region region; assert (is_power_of_2 (alignment)); - region.start = virtual_size (regions); + region.start = virtual_size (rs); if (IS_ALIGNED (region.start, alignment)) return 0; /* nothing to do */ region.end = (region.start & ~(alignment-1)) + alignment - 1; region.len = region.end - region.start + 1; region.type = region_zero; region.description = "padding"; - return append_one_region (regions, region); + return append_one_region (rs, region); } int -append_region_len (struct regions *regions, +append_region_len (regions *rs, const char *description, uint64_t len, uint64_t pre_aligment, uint64_t post_alignment, enum region_type type, ...) @@ -138,14 +130,14 @@ append_region_len (struct regions *regions, /* Pre-alignment. */ if (pre_aligment != 0) { - if (append_padding (regions, pre_aligment) == -1) + if (append_padding (rs, pre_aligment) == -1) return -1; - assert (IS_ALIGNED (virtual_size (regions), pre_aligment)); + assert (IS_ALIGNED (virtual_size (rs), pre_aligment)); } /* Main region. */ region.description = description; - region.start = virtual_size (regions); + region.start = virtual_size (rs); region.len = len; region.end = region.start + region.len - 1; region.type = type; @@ -167,14 +159,14 @@ append_region_len (struct regions *regions, va_end (ap); region.u.data = data; } - if (append_one_region (regions, region) == -1) + if (append_one_region (rs, region) == -1) return -1; /* Post-alignment. */ if (post_alignment != 0) { - if (append_padding (regions, post_alignment) == -1) + if (append_padding (rs, post_alignment) == -1) return -1; - assert (IS_ALIGNED (virtual_size (regions), post_alignment)); + assert (IS_ALIGNED (virtual_size (rs), post_alignment)); } return 0; diff --git a/plugins/partitioning/virtual-disk.h b/plugins/partitioning/virtual-disk.h index e1739783..4428ff17 100644 --- a/plugins/partitioning/virtual-disk.h +++ b/plugins/partitioning/virtual-disk.h @@ -91,7 +91,7 @@ struct file { extern struct file *files; extern size_t nr_files; -extern struct regions regions; +extern regions the_regions; extern unsigned char *primary, *secondary, **ebr; /* Main entry point called after files array has been populated. */ diff --git a/plugins/linuxdisk/partition-gpt.c b/plugins/linuxdisk/partition-gpt.c index 81a530d6..6380dc8f 100644 --- a/plugins/linuxdisk/partition-gpt.c +++ b/plugins/linuxdisk/partition-gpt.c @@ -197,7 +197,7 @@ create_gpt_partition_table (struct virtual_disk *disk, unsigned char *out) size_t j; for (j = 0; j < nr_regions (&disk->regions); ++j) { - const struct region *region = get_region (&disk->regions, j); + const struct region *region = &disk->regions.ptr[j]; /* Find the (only) partition region, which has type region_file. */ if (region->type == region_file) { diff --git a/plugins/partitioning/partition-gpt.c b/plugins/partitioning/partition-gpt.c index 75b4643a..b2843414 100644 --- a/plugins/partitioning/partition-gpt.c +++ b/plugins/partitioning/partition-gpt.c @@ -88,7 +88,7 @@ create_gpt_partition_header (const void *pt, bool is_primary, uint64_t nr_lbas; struct gpt_header *header = (struct gpt_header *) out; - nr_lbas = virtual_size (&regions) / SECTOR_SIZE; + nr_lbas = virtual_size (&the_regions) / SECTOR_SIZE; memset (header, 0, sizeof *header); memcpy (header->signature, GPT_SIGNATURE, sizeof (header->signature)); @@ -122,8 +122,8 @@ create_gpt_partition_table (unsigned char *out) { size_t i, j; - for (j = 0; j < nr_regions (&regions); ++j) { - const struct region *region = get_region (&regions, j); + for (j = 0; j < nr_regions (&the_regions); ++j) { + const struct region *region = &the_regions.ptr[j]; if (region->type == region_file) { i = region->u.i; @@ -188,7 +188,7 @@ create_gpt_protective_mbr (unsigned char *out) * expressible with MBR. */ region.start = 512; - end = virtual_size (&regions) - 1; + end = virtual_size (&the_regions) - 1; if (end > UINT32_MAX * SECTOR_SIZE) end = UINT32_MAX * SECTOR_SIZE; region.end = end; diff --git a/plugins/partitioning/partition-mbr.c b/plugins/partitioning/partition-mbr.c index 971e694f..5c18966c 100644 --- a/plugins/partitioning/partition-mbr.c +++ b/plugins/partitioning/partition-mbr.c @@ -90,7 +90,7 @@ create_mbr_layout (void) */ eptr0 = find_ebr_region (3, &j); region.start = eptr0->start; - region.end = virtual_size (&regions) - 1; /* to end of disk */ + region.end = virtual_size (&the_regions) - 1; /* to end of disk */ region.len = region.end - region.start + 1; create_mbr_partition_table_entry (&region, false, 0xf, &primary[0x1ee]); @@ -144,8 +144,8 @@ find_file_region (size_t i, size_t *j) { const struct region *region; - for (; *j < nr_regions (&regions); ++(*j)) { - region = get_region (&regions, *j); + for (; *j < nr_regions (&the_regions); ++(*j)) { + region = &the_regions.ptr[*j]; if (region->type == region_file && region->u.i == i) return region; } @@ -162,8 +162,8 @@ find_ebr_region (size_t i, size_t *j) assert (i >= 3); - for (; *j < nr_regions (&regions); ++(*j)) { - region = get_region (&regions, *j); + for (; *j < nr_regions (&the_regions); ++(*j)) { + region = &the_regions.ptr[*j]; if (region->type == region_data && region->u.data == ebr[i-3]) return region; } diff --git a/plugins/partitioning/partitioning.c b/plugins/partitioning/partitioning.c index 865acd28..33379569 100644 --- a/plugins/partitioning/partitioning.c +++ b/plugins/partitioning/partitioning.c @@ -79,7 +79,7 @@ struct file *files = NULL; size_t nr_files = 0; /* Virtual disk layout. */ -struct regions regions; +regions the_regions; /* Primary and secondary partition tables and extended boot records. * Secondary PT is only used for GPT. EBR array of sectors is only @@ -93,7 +93,7 @@ static struct random_state random_state; static void partitioning_load (void) { - init_regions (&regions); + init_regions (&the_regions); parse_guid (DEFAULT_TYPE_GUID, type_guid); xsrandom (time (NULL), &random_state); } @@ -110,7 +110,7 @@ partitioning_unload (void) /* We don't need to free regions.regions[].u.data because it points * to primary, secondary or ebr which we free here. */ - free_regions (&regions); + free_regions (&the_regions); free (primary); free (secondary); @@ -295,7 +295,7 @@ partitioning_open (int readonly) static int64_t partitioning_get_size (void *handle) { - return virtual_size (&regions); + return virtual_size (&the_regions); } /* Serves the same data over multiple connections. */ @@ -318,7 +318,7 @@ static int partitioning_pread (void *handle, void *buf, uint32_t count, uint64_t offset) { while (count > 0) { - const struct region *region = find_region (&regions, offset); + const struct region *region = find_region (&the_regions, offset); size_t i, len; ssize_t r; @@ -366,7 +366,7 @@ partitioning_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset) { while (count > 0) { - const struct region *region = find_region (&regions, offset); + const struct region *region = find_region (&the_regions, offset); size_t i, len; ssize_t r; diff --git a/plugins/partitioning/virtual-disk.c b/plugins/partitioning/virtual-disk.c index bb97d007..5a16fb04 100644 --- a/plugins/partitioning/virtual-disk.c +++ b/plugins/partitioning/virtual-disk.c @@ -55,7 +55,7 @@ create_virtual_disk_layout (void) { size_t i; - assert (nr_regions (&regions) == 0); + assert (nr_regions (&the_regions) == 0); assert (nr_files > 0); assert (primary == NULL); assert (secondary == NULL); @@ -104,13 +104,13 @@ create_virtual_disk_layout (void) /* Virtual primary partition table region at the start of the disk. */ if (parttype == PARTTYPE_MBR) { - if (append_region_len (&regions, "MBR", + if (append_region_len (&the_regions, "MBR", SECTOR_SIZE, 0, 0, region_data, primary) == -1) return -1; } else /* PARTTYPE_GPT */ { - if (append_region_len (&regions, "GPT primary", + if (append_region_len (&the_regions, "GPT primary", (2+GPT_PTA_LBAs) * SECTOR_SIZE, 0, 0, region_data, primary) == -1) return -1; @@ -120,7 +120,7 @@ create_virtual_disk_layout (void) for (i = 0; i < nr_files; ++i) { uint64_t offset; - offset = virtual_size (&regions); + offset = virtual_size (&the_regions); /* Because we add padding after each partition, this invariant * must always be true. */ @@ -128,7 +128,7 @@ create_virtual_disk_layout (void) /* Logical partitions are preceeded by an EBR. */ if (parttype == PARTTYPE_MBR && nr_files > 4 && i >= 3) { - if (append_region_len (&regions, "EBR", + if (append_region_len (&the_regions, "EBR", SECTOR_SIZE, 0, 0, region_data, ebr[i-3]) == -1) return -1; @@ -139,7 +139,7 @@ create_virtual_disk_layout (void) * If the file size is not a multiple of SECTOR_SIZE then * add a padding region at the end to round it up. */ - if (append_region_len (&regions, files[i].filename, + if (append_region_len (&the_regions, files[i].filename, files[i].statbuf.st_size, files[i].alignment, SECTOR_SIZE, region_file, i) == -1) @@ -148,15 +148,15 @@ create_virtual_disk_layout (void) /* For GPT add the virtual secondary/backup partition table. */ if (parttype == PARTTYPE_GPT) { - if (append_region_len (&regions, "GPT secondary", + if (append_region_len (&the_regions, "GPT secondary", (GPT_PTA_LBAs+1) * SECTOR_SIZE, 0, 0, region_data, secondary) == -1) return -1; } if (partitioning_debug_regions) { - for (i = 0; i < nr_regions (&regions); ++i) { - const struct region *region = get_region (&regions, i); + for (i = 0; i < nr_regions (&the_regions); ++i) { + const struct region *region = &the_regions.ptr[i]; nbdkit_debug ("region[%zu]: %" PRIx64 "-%" PRIx64 " type=%s", i, region->start, region->end, @@ -168,13 +168,13 @@ create_virtual_disk_layout (void) } /* We must have created some regions. */ - assert (nr_regions (&regions) > 0); + assert (nr_regions (&the_regions) > 0); /* Check the final alignment of all the partitions is the same as * what was requested. */ - for (i = 0; i < nr_regions (&regions); ++i) { - const struct region *region = get_region (&regions, i); + for (i = 0; i < nr_regions (&the_regions); ++i) { + const struct region *region = &the_regions.ptr[i]; if (region->type == region_file) assert (IS_ALIGNED (region->start, files[region->u.i].alignment)); @@ -189,7 +189,7 @@ create_partition_table (void) /* The caller has already created the disk layout and allocated * space in memory for the partition table. */ - assert (nr_regions (&regions) > 0); + assert (nr_regions (&the_regions) > 0); assert (primary != NULL); if (parttype == PARTTYPE_GPT) assert (secondary != NULL); -- 2.25.0