Eric Blake <ebl...@redhat.com> writes: > On 12/17/2015 09:49 AM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> contrib/ivshmem-server/main.c | 4 +--- >> qdev-monitor.c | 3 +-- >> qemu-nbd.c | 3 +-- >> 3 files changed, 3 insertions(+), 7 deletions(-) >> >> diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c >> index 54ff001..00508b5 100644 >> --- a/contrib/ivshmem-server/main.c >> +++ b/contrib/ivshmem-server/main.c >> @@ -106,9 +106,7 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int >> argc, char *argv[]) >> case 'l': /* shm_size */ >> parse_option_size("shm_size", optarg, &args->shm_size, &errp); > > Idea for a followup patch: The name 'errp' is most often associated with > type 'Error **'; but here it is 'Error *', which is confusing.
Yes. Another round of these: e940f54 qmp hmp: Consistently name Error * objects err, and not errp 2f719f1 hw: Consistently name Error * objects err, and not errp 4399c43 qemu-img: Consistently name Error * objects err, and not errp > Other offenders in hmp.c, hw/core/nmi.c, include/qemu/sockets.h, and > tests/test-string-output-visitor.c. > >> if (errp) { >> - fprintf(stderr, "cannot parse shm size: %s\n", >> - error_get_pretty(errp)); >> - error_free(errp); >> + error_report_err(errp); > > This loses the "cannot parse shm size: " prefix; but I don't think that > hurts. Could use a mention in the commit message, though. Losing the prefix is fine, because the error messages always mention the parameter name. For instance, cannot parse shm size: Parameter 'shm_size' expects a size becomes Parameter 'shm_size' expects a size You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and terabytes. Note we now get the hint. Including the error location would be even nicer, but is out of scope for this series. The program shows its complete usage help afterwards, which is annoying, but out of scope for this series. I'll amend the commit message. > Reviewed-by: Eric Blake <ebl...@redhat.com> Thanks!