On Thu, Jun 30, 2022 at 05:38:14PM +0200, Laszlo Ersek wrote:
On 06/29/22 15:36, Richard W.M. Jones wrote:
+ * The invariant for '*i' is always an extent which starts before or + * equal to the current offset. + */ +static bool +only_zeroes (const extent_list exts, size_t *i,
(5) I think it's a bit wasteful (or at least strange) to pass in the entire vector structure; we certainly don't need the "cap" field. Other functions in libnbd seem to take pointers to vectors (which is what I'd expect).
Note it's only passing in 3 fields (24 bytes), not the entire array (and I guess it should inline the whole function). Does that change things?
@@ -177,7 +229,10 @@ worker_thread (void *wp) extent_list exts = empty_vector;
while (get_next_offset (&offset, &count)) { + struct command *command; size_t i; + bool is_zeroing = false; + uint64_t zeroing_start = 0;
(14) I think we should not initialize "zeroing_start". It suggests that the initializer value is meaningful, but it is not.
GCC warns if you don't initialize it! It doesn't understand that is_zeroing is related to the validity of zeroing_start. Thanks for the other suggestions, I'll rework things. 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