| Message ID | 20231030133712.2822276-5-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | dump: Minor fixes & improvements | expand |
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; >> }
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; }
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(-)