[Libguestfs] [PATCH nbdkit 4/9] common/regions: Use new vector type to store the list of regions.
Richard W.M. Jones
rjones at redhat.com
Wed Apr 15 16:16:45 UTC 2020
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 ®ions->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 (®ions) / 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 (®ions); ++j) {
- const struct region *region = get_region (®ions, 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 (®ions) - 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 (®ions) - 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 (®ion, 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 (®ions); ++(*j)) {
- region = get_region (®ions, *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 (®ions); ++(*j)) {
- region = get_region (®ions, *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 (®ions);
+ 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 (®ions);
+ 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 (®ions);
+ 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 (®ions, 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 (®ions, 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 (®ions) == 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 (®ions, "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 (®ions, "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 (®ions);
+ 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 (®ions, "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 (®ions, 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 (®ions, "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 (®ions); ++i) {
- const struct region *region = get_region (®ions, 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 (®ions) > 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 (®ions); ++i) {
- const struct region *region = get_region (®ions, 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 (®ions) > 0);
+ assert (nr_regions (&the_regions) > 0);
assert (primary != NULL);
if (parttype == PARTTYPE_GPT)
assert (secondary != NULL);
--
2.25.0
More information about the Libguestfs
mailing list