Re: [Qemu-devel] [PATCH v4 05/14] contrib/ivshmem-*: switch to QEMU headers

[Eric Blake]

On 09/02/2014 09:25 AM, David Marchand wrote:
> Reuse parsers from QEMU, C99 boolean.
> 
> Signed-off-by: David Marchand <address@hidden>
> ---
>  contrib/ivshmem-client/ivshmem-client.c |   12 +----
>  contrib/ivshmem-client/ivshmem-client.h |    4 +-
>  contrib/ivshmem-client/main.c           |   12 +----
>  contrib/ivshmem-server/ivshmem-server.c |   14 +-----
>  contrib/ivshmem-server/ivshmem-server.h |    4 +-
>  contrib/ivshmem-server/main.c           |   73 
> +++++--------------------------
>  6 files changed, 20 insertions(+), 99 deletions(-)

Why introduce code in patch 1 only to rip it back out in patch 5?  Why
not just squash these together and just introduce the contrib file
correct on its first commit?


>  
>          case 'l': /* shm_size */
> -            if (parse_size(optarg, &args->shm_size) < 0) {
> +            parse_option_size("shm_size", optarg, &args->shm_size, &errp);
> +            if (errp) {
> +                error_free(errp);
>                  fprintf(stderr, "cannot parse shm size\n");

It would be nicer to print the contents of errp, instead of discarding
what is likely to be a more specific error message.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature