Zero @length is rejected with "Invalid parameter 'length'".  Improve
to "Parameter 'length' expects a non-zero length".

@protocol values not starting with "fd:" or "file:" are rejected with
"Invalid parameter 'protocol'".  Improve to "parameter 'protocol' must
start with 'file:' or 'fd:'".

While there, make the conditional checking @protocol a little more
obvious.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 dump/dump.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index a5e9a06ef1..d888e4bd3c 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -1812,7 +1812,8 @@ static void dump_init(DumpState *s, int fd, bool has_format,
 
     s->fd = fd;
     if (has_filter && !length) {
-        error_setg(errp, QERR_INVALID_PARAMETER, "length");
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "length",
+                   "a non-zero size");
         goto cleanup;
     }
     s->filter_area_begin = begin;
@@ -2072,7 +2073,7 @@ void qmp_dump_guest_memory(bool paging, const char *protocol,
 {
     ERRP_GUARD();
     const char *p;
-    int fd = -1;
+    int fd;
     DumpState *s;
     bool detach_p = false;
 
@@ -2135,18 +2136,15 @@ void qmp_dump_guest_memory(bool paging, const char *protocol,
         if (fd == -1) {
             return;
         }
-    }
-
-    if  (strstart(protocol, "file:", &p)) {
+    } else if  (strstart(protocol, "file:", &p)) {
         fd = qemu_open_old(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
         if (fd < 0) {
             error_setg_file_open(errp, errno, p);
             return;
         }
-    }
-
-    if (fd == -1) {
-        error_setg(errp, QERR_INVALID_PARAMETER, "protocol");
+    } else {
+        error_setg(errp,
+                   "parameter 'protocol' must start with 'file:' or 'fd:'");
         return;
     }
 
-- 
2.41.0

Hi Markus,

On 30/10/23 14:37, Markus Armbruster wrote:
> Zero @length is rejected with "Invalid parameter 'length'".  Improve
> to "Parameter 'length' expects a non-zero length".
> 
> @protocol values not starting with "fd:" or "file:" are rejected with
> "Invalid parameter 'protocol'".  Improve to "parameter 'protocol' must
> start with 'file:' or 'fd:'".
> 
> While there, make the conditional checking @protocol a little more
> obvious.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   dump/dump.c | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/dump/dump.c b/dump/dump.c
> index a5e9a06ef1..d888e4bd3c 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -1812,7 +1812,8 @@ static void dump_init(DumpState *s, int fd, bool has_format,
>   
>       s->fd = fd;
>       if (has_filter && !length) {
> -        error_setg(errp, QERR_INVALID_PARAMETER, "length");
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "length",

Per commit 4629ed1e98 ("qerror: Finally unused, clean up", 2015):

  /*
   * These macros will go away, please don't use in new code, ...

Instead we can use:

            error_setg(errp, "Parameter '%s' expects %s", "length",

> +                   "a non-zero size");
>           goto cleanup;
>       }

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Hi Markus,
>
> On 30/10/23 14:37, Markus Armbruster wrote:
>> Zero @length is rejected with "Invalid parameter 'length'".  Improve
>> to "Parameter 'length' expects a non-zero length".
>>
>> @protocol values not starting with "fd:" or "file:" are rejected with
>> "Invalid parameter 'protocol'".  Improve to "parameter 'protocol' must
>> start with 'file:' or 'fd:'".
>>
>> While there, make the conditional checking @protocol a little more
>> obvious.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   dump/dump.c | 16 +++++++---------
>>   1 file changed, 7 insertions(+), 9 deletions(-)
>> diff --git a/dump/dump.c b/dump/dump.c
>> index a5e9a06ef1..d888e4bd3c 100644
>> --- a/dump/dump.c
>> +++ b/dump/dump.c
>> @@ -1812,7 +1812,8 @@ static void dump_init(DumpState *s, int fd, bool has_format,
>>         s->fd = fd;
>>       if (has_filter && !length) {
>> -        error_setg(errp, QERR_INVALID_PARAMETER, "length");
>> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "length",
>
> Per commit 4629ed1e98 ("qerror: Finally unused, clean up", 2015):
>
>  /*
>   * These macros will go away, please don't use in new code, ...
>
> Instead we can use:
>
>            error_setg(errp, "Parameter '%s' expects %s", "length",

I left this to the next version of your "qapi: Kill 'qapi/qmp/qerror.h'
for good" out of laziness.  Since you prefer the deed to be done right
away, I will in v2.

>> +                   "a non-zero size");
>>           goto cleanup;
>>       }