Re: [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 
> ---
>  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

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

[David Marchand]

Reuse parsers from QEMU, C99 boolean.

Signed-off-by: David Marchand 
---
 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(-)

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index ce3a5d2..2ba40a7 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -6,19 +6,11 @@
  * top-level directory.
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
 #include 
 #include 
 #include 
 
+#include "qemu-common.h"
 #include "qemu/queue.h"
 
 #include "ivshmem-client.h"
@@ -149,7 +141,7 @@ handle_server_msg(IvshmemClient *client)
 int
 ivshmem_client_init(IvshmemClient *client, const char *unix_sock_path,
 ivshmem_client_notif_cb_t notif_cb, void *notif_arg,
-int verbose)
+bool verbose)
 {
 unsigned i;
 
diff --git a/contrib/ivshmem-client/ivshmem-client.h 
b/contrib/ivshmem-client/ivshmem-client.h
index e3b284d..45f2b64 100644
--- a/contrib/ivshmem-client/ivshmem-client.h
+++ b/contrib/ivshmem-client/ivshmem-client.h
@@ -78,7 +78,7 @@ struct IvshmemClient {
 ivshmem_client_notif_cb_t notif_cb; /**< notification callback */
 void *notif_arg;/**< notification argument */
 
-int verbose;/**< true to enable debug */
+bool verbose;   /**< true to enable debug */
 };
 
 /**
@@ -101,7 +101,7 @@ struct IvshmemClient {
  */
 int ivshmem_client_init(IvshmemClient *client, const char *unix_sock_path,
 ivshmem_client_notif_cb_t notif_cb, void *notif_arg,
-int verbose);
+bool verbose);
 
 /**
  * Connect to the server
diff --git a/contrib/ivshmem-client/main.c b/contrib/ivshmem-client/main.c
index 778d0f2..f8a7b66 100644
--- a/contrib/ivshmem-client/main.c
+++ b/contrib/ivshmem-client/main.c
@@ -6,15 +6,7 @@
  * top-level directory.
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include "qemu-common.h"
 
 #include "ivshmem-client.h"
 
@@ -22,7 +14,7 @@
 #define DEFAULT_UNIX_SOCK_PATH "/tmp/ivshmem_socket"
 
 typedef struct IvshmemClientArgs {
-int verbose;
+bool verbose;
 char *unix_sock_path;
 } IvshmemClientArgs;
 
diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index e58864d..0afa6e8 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -6,23 +6,13 @@
  * top-level directory.
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
 #include 
-#include 
 #include 
 #include 
 #include 
 #include 
 
+#include "qemu-common.h"
 #include "qemu/queue.h"
 
 #include "ivshmem-server.h"
@@ -246,7 +236,7 @@ ivshmem_ftruncate(int fd, unsigned shmsize)
 int
 ivshmem_server_init(IvshmemServer *server, const char *unix_sock_path,
 const char *shm_path, size_t shm_size, unsigned n_vectors,
-int verbose)
+bool verbose)
 {
 memset(server, 0, sizeof(*server));
 
diff --git a/contrib/ivshmem-server/ivshmem-server.h 
b/contrib/ivshmem-server/ivshmem-server.h
index acd17a8..5ccc7af 100644
--- a/contrib/ivshmem-server/ivshmem-server.h
+++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -69,7 +69,7 @@ typedef struct IvshmemServer {
 int shm_fd;  /**< shm file descriptor */
 unsigned n_vectors;  /**< number of vectors */
 long cur_id; /**< id to be given to next client */
-int verbose; /**< true in verbose mode */
+bool verbose;/**< true in verbose mode */
 IvshmemServerPeerList peer_list; /**< list of peers */
 } IvshmemServer;
 
@@ -97,7 +97,7 @@ typedef struct IvshmemServer {
 int
 ivshmem_server_init(IvshmemServer *server, const char *unix_sock_path,
 const char *shm_path, size_t shm_size, unsigned n_vectors,
-int verbose);
+bool verbose);
 
 /**
  * Open the shm, then create and bind to the unix socket
diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
index a4504c3..f00e6f9 100644
--- a/contrib/ivshmem-server/main.c
+++ b/contrib/ivshmem-server/main.c
@@ -6,17 +6,7 @@
  * top-level directory.
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include "qemu-common.h"
 
 #include "ivshmem-server.h"
 
@@ -30,8