On 8/29/22 22:17, Janis Schoetterl-Glausch wrote:
On Thu, 2022-08-11 at 12:10 +0000, Janosch Frank wrote:
get_start_block() returns the start address of the first memory block
or -1.

With the GuestPhysBlock iterator conversion we don't need to set the
start address and can therefore remove that code and the "start"
DumpState struct member. The only functionality left is the validation
of the start block so it only makes sense to re-name the function to
validate_start_block()

Nit, since you don't return an address anymore, I find retaining the -
1/0 return value instead of true/false weird.

I'm trying to wrap up fixing things for a new version.
My fix for this is calling it is_start_block_valid() and making the return bool. That being said, having int for true/false is not uncommon in C but explicitly checking -1 makes it look weird.



diff --git i/dump/dump.c w/dump/dump.c
index e204912a89..2239bd324e 100644
--- i/dump/dump.c
+++ w/dump/dump.c
@@ -1500,12 +1500,12 @@ static void create_kdump_vmcore(DumpState *s, Error **errp)
     }
 }

-static int validate_start_block(DumpState *s)
+static bool is_start_block_valid(DumpState *s)
 {
     GuestPhysBlock *block;

     if (!s->has_filter) {
-        return 0;
+        return true;
     }

     QTAILQ_FOREACH(block, &s->guest_phys_blocks.head, next) {
@@ -1514,10 +1514,10 @@ static int validate_start_block(DumpState *s)
             block->target_end <= s->begin) {
             continue;
         }
-        return 0;
+        return true;
    }

-    return -1;
+    return false;
 }

 static void get_max_mapnr(DumpState *s)
@@ -1663,7 +1663,7 @@ static void dump_init(DumpState *s, int fd, bool has_format,
     }

     /* Is the filter filtering everything? */
-    if (validate_start_block(s) == -1) {
+    if (!is_start_block_valid(s)) {
         error_setg(errp, QERR_INVALID_PARAMETER, "begin");
         goto cleanup;
     }


Reply via email to