From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40568) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLUPl-0007oQ-4v for qemu-devel@nongnu.org; Tue, 19 Jan 2016 06:25:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLUPc-0004XJ-9H for qemu-devel@nongnu.org; Tue, 19 Jan 2016 06:25:33 -0500 Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:58659) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLUPb-0004WE-TM for qemu-devel@nongnu.org; Tue, 19 Jan 2016 06:25:24 -0500 Received: from localhost by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 19 Jan 2016 11:25:19 -0000 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 56E9B2190023 for ; Tue, 19 Jan 2016 11:25:05 +0000 (GMT) Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com [9.149.37.249]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u0JBPHEL28377274 for ; Tue, 19 Jan 2016 11:25:17 GMT Received: from d06av08.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u0JBPGxN002628 for ; Tue, 19 Jan 2016 04:25:17 -0700 Date: Tue, 19 Jan 2016 12:25:14 +0100 From: Greg Kurz Message-ID: <20160119122514.1ec96b1f@bahia.huguette.org> In-Reply-To: <87mvs2j16a.fsf@blackfin.pond.sub.org> References: <20160118150234.30699.57180.stgit@bahia.huguette.org> <20160118150240.30699.12563.stgit@bahia.huguette.org> <87mvs2j16a.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] 9pfs: use error_report() instead of fprintf(stderr) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, "Aneesh Kumar K.V" On Mon, 18 Jan 2016 17:35:25 +0100 Markus Armbruster wrote: > Greg Kurz writes: > > > Signed-off-by: Greg Kurz > > --- I agree to all your suggestions. Thanks ! > > hw/9pfs/9p-handle.c | 5 +++-- > > hw/9pfs/9p-local.c | 15 ++++++++------- > > hw/9pfs/9p-proxy.c | 12 ++++++------ > > hw/9pfs/9p.c | 2 +- > > 4 files changed, 18 insertions(+), 16 deletions(-) > > > > diff --git a/hw/9pfs/9p-handle.c b/hw/9pfs/9p-handle.c > > index 58b77b4c942d..8ba88775a2b6 100644 > > --- a/hw/9pfs/9p-handle.c > > +++ b/hw/9pfs/9p-handle.c > > @@ -19,6 +19,7 @@ > > #include > > #include > > #include "qemu/xattr.h" > > +#include "qemu/error-report.h" > > #include > > #include > > #ifdef CONFIG_LINUX_MAGIC_H > > @@ -655,12 +656,12 @@ static int handle_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) > > const char *path = qemu_opt_get(opts, "path"); > > > > if (sec_model) { > > - fprintf(stderr, "Invalid argument security_model specified with handle fsdriver\n"); > > + error_report("Invalid argument security_model specified with handle fsdriver"); > > return -1; > > } > > > > if (!path) { > > - fprintf(stderr, "fsdev: No path specified.\n"); > > + error_report("fsdev: No path specified."); > > Recommend to drop the period while there. > > > return -1; > > } > > fse->path = g_strdup(path); > > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > > index bf63eab729ad..9c25ab2db26b 100644 > > --- a/hw/9pfs/9p-local.c > > +++ b/hw/9pfs/9p-local.c > > @@ -20,6 +20,7 @@ > > #include > > #include > > #include "qemu/xattr.h" > > +#include "qemu/error-report.h" > > #include > > #include > > #ifdef CONFIG_LINUX_MAGIC_H > > @@ -1209,9 +1210,9 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) > > const char *path = qemu_opt_get(opts, "path"); > > > > if (!sec_model) { > > - fprintf(stderr, "security model not specified, " > > - "local fs needs security model\nvalid options are:" > > - "\tsecurity_model=[passthrough|mapped|none]\n"); > > + error_report("security model not specified, local fs needs security model"); > > + error_printf("valid options are:" > > + "\tsecurity_model=[passthrough|mapped-xattr|mapped-file|none]\n"); > > return -1; > > } > > > > @@ -1225,14 +1226,14 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) > > } else if (!strcmp(sec_model, "mapped-file")) { > > fse->export_flags |= V9FS_SM_MAPPED_FILE; > > } else { > > - fprintf(stderr, "Invalid security model %s specified, valid options are" > > - "\n\t [passthrough|mapped-xattr|mapped-file|none]\n", > > - sec_model); > > + error_report("Invalid security model %s specified, valid options are", > > + sec_model); > > + error_printf("\t [passthrough|mapped-xattr|mapped-file|none]\n"); > > Neater: > > error_report("Invalid security model %s specified", sec_model); > error_printf("valid options are;" > "\t[passthrough|mapped-xattr|mapped-file|none]\n"); > > > return -1; > > } > > > > if (!path) { > > - fprintf(stderr, "fsdev: No path specified.\n"); > > + error_report("fsdev: No path specified."); > > Recommend to drop the period while there. > > > return -1; > > } > > fse->path = g_strdup(path); > > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > > index 73d00dd74d11..72b9952d7c8b 100644 > > --- a/hw/9pfs/9p-proxy.c > > +++ b/hw/9pfs/9p-proxy.c > > @@ -1100,19 +1100,19 @@ static int connect_namedsocket(const char *path) > > struct sockaddr_un helper; > > > > if (strlen(path) >= sizeof(helper.sun_path)) { > > - fprintf(stderr, "Socket name too large\n"); > > + error_report("Socket name too large"); > > "too long" would be clearer, I think. > > > return -1; > > } > > sockfd = socket(AF_UNIX, SOCK_STREAM, 0); > > if (sockfd < 0) { > > - fprintf(stderr, "failed to create socket: %s\n", strerror(errno)); > > + error_report("failed to create socket: %s", strerror(errno)); > > return -1; > > } > > strcpy(helper.sun_path, path); > > helper.sun_family = AF_UNIX; > > size = strlen(helper.sun_path) + sizeof(helper.sun_family); > > if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) { > > - fprintf(stderr, "failed to connect to %s: %s\n", path, strerror(errno)); > > + error_report("failed to connect to %s: %s", path, strerror(errno)); > > close(sockfd); > > return -1; > > } > > @@ -1128,11 +1128,11 @@ static int proxy_parse_opts(QemuOpts *opts, struct FsDriverEntry *fs) > > const char *sock_fd = qemu_opt_get(opts, "sock_fd"); > > > > if (!socket && !sock_fd) { > > - fprintf(stderr, "socket and sock_fd none of the option specified\n"); > > + error_report("socket and sock_fd none of the option specified"); > > "Must specify either socket or sock_fd" would be clearer. > > > return -1; > > } > > if (socket && sock_fd) { > > - fprintf(stderr, "Both socket and sock_fd options specified\n"); > > + error_report("Both socket and sock_fd options specified"); > > return -1; > > } > > if (socket) { > > @@ -1155,7 +1155,7 @@ static int proxy_init(FsContext *ctx) > > } else { > > sock_id = atoi(ctx->fs_root); > > if (sock_id < 0) { > > - fprintf(stderr, "socket descriptor not initialized\n"); > > + error_report("socket descriptor not initialized"); > > } > > } > > if (sock_id < 0) { > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > index 3ff310605cd4..1f3bd12e0d0c 100644 > > --- a/hw/9pfs/9p.c > > +++ b/hw/9pfs/9p.c > > @@ -3370,7 +3370,7 @@ static void __attribute__((__constructor__)) v9fs_set_fd_limit(void) > > { > > struct rlimit rlim; > > if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) { > > - fprintf(stderr, "Failed to get the resource limit\n"); > > + error_report("Failed to get the resource limit"); > > exit(1); > > } > > open_fd_hw = rlim.rlim_cur - MIN(400, rlim.rlim_cur/3); >