| Message ID | 1411082250-27776-1-git-send-email-drjones@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 09/19/2014 01:17 AM, Andrew Jones wrote: > Replace all the fprintf(stderr, ...) calls with error_report. > Also make sure exit() consistently uses the error code 1. A few calls > used -1. > > Signed-off-by: Andrew Jones <drjones@redhat.com> > --- > hw/misc/ivshmem.c | 39 +++++++++++++++++++-------------------- > 1 file changed, 19 insertions(+), 20 deletions(-) > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > index bf585b7691998..b3983296f58fa 100644 > --- a/hw/misc/ivshmem.c > +++ b/hw/misc/ivshmem.c > @@ -300,8 +300,8 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * > chr = qemu_chr_open_eventfd(eventfd); > > if (chr == NULL) { > - fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); > - exit(-1); > + error_report("creating eventfd for eventfd %d failed", eventfd); > + exit(1); > } > qemu_chr_fe_claim_no_fail(chr); > > @@ -328,15 +328,14 @@ static int check_shm_size(IVShmemState *s, int fd) { > struct stat buf; > > if (fstat(fd, &buf) < 0) { > - fprintf(stderr, "ivshmem: exiting: fstat on fd %d failed: %s\n", > + error_report("exiting: fstat on fd %d failed: %s", > fd, strerror(errno)); > return -1; > } > > if (s->ivshmem_size > buf.st_size) { > - fprintf(stderr, > - "IVSHMEM ERROR: Requested memory size greater" > - " than shared object size (%" PRIu64 " > %" PRIu64")\n", > + error_report("Requested memory size greater" > + " than shared object size (%" PRIu64 " > %" PRIu64")", > s->ivshmem_size, (uint64_t)buf.st_size); > return -1; > } else { > @@ -510,7 +509,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > incoming_fd = dup(tmp_fd); > > if (incoming_fd == -1) { > - fprintf(stderr, "could not allocate file descriptor %s\n", > + error_report("could not allocate file descriptor %s", > strerror(errno)); > close(tmp_fd); > return; > @@ -524,7 +523,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > s->max_peer = 0; > > if (check_shm_size(s, incoming_fd) == -1) { > - exit(-1); > + exit(1); > } > > /* mmap the region and map into the BAR2 */ > @@ -618,13 +617,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { > value <<= 30; > break; > default: > - fprintf(stderr, "qemu: invalid ram size: %s\n", s->sizearg); > + error_report("invalid ram size: %s", s->sizearg); > exit(1); > } > > /* BARs must be a power of 2 */ > if (!is_power_of_two(value)) { > - fprintf(stderr, "ivshmem: size must be power of 2\n"); > + error_report("size must be power of 2"); > exit(1); > } > > @@ -676,7 +675,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) > } > > if (proxy->role_val == IVSHMEM_PEER) { > - fprintf(stderr, "ivshmem: 'peer' devices are not migratable\n"); > + error_report("'peer' devices are not migratable"); > return -EINVAL; > } > > @@ -722,7 +721,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > /* IRQFD requires MSI */ > if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && > !ivshmem_has_feature(s, IVSHMEM_MSI)) { > - fprintf(stderr, "ivshmem: ioeventfd/irqfd requires MSI\n"); > + error_report("ioeventfd/irqfd requires MSI"); > exit(1); > } > > @@ -733,7 +732,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > } else if (strncmp(s->role, "master", 7) == 0) { > s->role_val = IVSHMEM_MASTER; > } else { > - fprintf(stderr, "ivshmem: 'role' must be 'peer' or 'master'\n"); > + error_report("'role' must be 'peer' or 'master'"); > exit(1); > } > } else { > @@ -773,8 +772,8 @@ static int pci_ivshmem_init(PCIDevice *dev) > * to the ivshmem server to receive the memory region */ > > if (s->shmobj != NULL) { > - fprintf(stderr, "WARNING: do not specify both 'chardev' " > - "and 'shm' with ivshmem\n"); > + error_report("WARNING: do not specify both 'chardev' " > + "and 'shm' with ivshmem"); > } > > IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", > @@ -802,7 +801,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > int fd; > > if (s->shmobj == NULL) { > - fprintf(stderr, "Must specify 'chardev' or 'shm' to ivshmem\n"); > + error_report("Must specify 'chardev' or 'shm' to ivshmem"); > exit(1); > } > > @@ -814,18 +813,18 @@ static int pci_ivshmem_init(PCIDevice *dev) > S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { > /* truncate file to length PCI device's memory */ > if (ftruncate(fd, s->ivshmem_size) != 0) { > - fprintf(stderr, "ivshmem: could not truncate shared file\n"); > + error_report("could not truncate shared file"); > } > > } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, > S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { > - fprintf(stderr, "ivshmem: could not open shared file\n"); > - exit(-1); > + error_report("could not open shared file"); > + exit(1); > > } > > if (check_shm_size(s, fd) == -1) { > - exit(-1); > + exit(1); > } > > create_shared_memory_BAR(s, fd); > Looks good to me (a comment on a dependency on Andreas patchset would have been good). Acked-by: David Marchand <david.marchand@6wind.com>
On 2014/9/19 7:17, Andrew Jones wrote: > Replace all the fprintf(stderr, ...) calls with error_report. > Also make sure exit() consistently uses the error code 1. A few calls > used -1. > > Signed-off-by: Andrew Jones <drjones@redhat.com> > --- > hw/misc/ivshmem.c | 39 +++++++++++++++++++-------------------- > 1 file changed, 19 insertions(+), 20 deletions(-) > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > index bf585b7691998..b3983296f58fa 100644 > --- a/hw/misc/ivshmem.c > +++ b/hw/misc/ivshmem.c > @@ -300,8 +300,8 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * > chr = qemu_chr_open_eventfd(eventfd); > > if (chr == NULL) { > - fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); > - exit(-1); > + error_report("creating eventfd for eventfd %d failed", eventfd); > + exit(1); > } > qemu_chr_fe_claim_no_fail(chr); > > @@ -328,15 +328,14 @@ static int check_shm_size(IVShmemState *s, int fd) { > struct stat buf; > > if (fstat(fd, &buf) < 0) { > - fprintf(stderr, "ivshmem: exiting: fstat on fd %d failed: %s\n", > + error_report("exiting: fstat on fd %d failed: %s", > fd, strerror(errno)); The indentation looks weird, better to fix it.;) More of the same elsewhere. > return -1; > } > > if (s->ivshmem_size > buf.st_size) { > - fprintf(stderr, > - "IVSHMEM ERROR: Requested memory size greater" > - " than shared object size (%" PRIu64 " > %" PRIu64")\n", > + error_report("Requested memory size greater" > + " than shared object size (%" PRIu64 " > %" PRIu64")", > s->ivshmem_size, (uint64_t)buf.st_size); > return -1; > } else { > @@ -510,7 +509,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > incoming_fd = dup(tmp_fd); > > if (incoming_fd == -1) { > - fprintf(stderr, "could not allocate file descriptor %s\n", > + error_report("could not allocate file descriptor %s", > strerror(errno)); > close(tmp_fd); > return; > @@ -524,7 +523,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > s->max_peer = 0; > > if (check_shm_size(s, incoming_fd) == -1) { > - exit(-1); > + exit(1); > } > > /* mmap the region and map into the BAR2 */ > @@ -618,13 +617,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { > value <<= 30; > break; > default: > - fprintf(stderr, "qemu: invalid ram size: %s\n", s->sizearg); > + error_report("invalid ram size: %s", s->sizearg); > exit(1); > } > > /* BARs must be a power of 2 */ > if (!is_power_of_two(value)) { > - fprintf(stderr, "ivshmem: size must be power of 2\n"); > + error_report("size must be power of 2"); > exit(1); > } > > @@ -676,7 +675,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) > } > > if (proxy->role_val == IVSHMEM_PEER) { > - fprintf(stderr, "ivshmem: 'peer' devices are not migratable\n"); > + error_report("'peer' devices are not migratable"); > return -EINVAL; > } > > @@ -722,7 +721,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > /* IRQFD requires MSI */ > if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && > !ivshmem_has_feature(s, IVSHMEM_MSI)) { > - fprintf(stderr, "ivshmem: ioeventfd/irqfd requires MSI\n"); > + error_report("ioeventfd/irqfd requires MSI"); > exit(1); > } > > @@ -733,7 +732,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > } else if (strncmp(s->role, "master", 7) == 0) { > s->role_val = IVSHMEM_MASTER; > } else { > - fprintf(stderr, "ivshmem: 'role' must be 'peer' or 'master'\n"); > + error_report("'role' must be 'peer' or 'master'"); > exit(1); > } > } else { > @@ -773,8 +772,8 @@ static int pci_ivshmem_init(PCIDevice *dev) > * to the ivshmem server to receive the memory region */ > > if (s->shmobj != NULL) { > - fprintf(stderr, "WARNING: do not specify both 'chardev' " > - "and 'shm' with ivshmem\n"); > + error_report("WARNING: do not specify both 'chardev' " > + "and 'shm' with ivshmem"); > } > > IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", > @@ -802,7 +801,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > int fd; > > if (s->shmobj == NULL) { > - fprintf(stderr, "Must specify 'chardev' or 'shm' to ivshmem\n"); > + error_report("Must specify 'chardev' or 'shm' to ivshmem"); > exit(1); > } > > @@ -814,18 +813,18 @@ static int pci_ivshmem_init(PCIDevice *dev) > S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { > /* truncate file to length PCI device's memory */ > if (ftruncate(fd, s->ivshmem_size) != 0) { > - fprintf(stderr, "ivshmem: could not truncate shared file\n"); > + error_report("could not truncate shared file"); > } > > } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, > S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { > - fprintf(stderr, "ivshmem: could not open shared file\n"); > - exit(-1); > + error_report("could not open shared file"); > + exit(1); > > } > > if (check_shm_size(s, fd) == -1) { > - exit(-1); > + exit(1); > } > > create_shared_memory_BAR(s, fd); >
On 2014/9/19 15:34, zhanghailiang wrote: > On 2014/9/19 7:17, Andrew Jones wrote: >> Replace all the fprintf(stderr, ...) calls with error_report. >> Also make sure exit() consistently uses the error code 1. A few calls >> used -1. >> >> Signed-off-by: Andrew Jones <drjones@redhat.com> >> --- >> hw/misc/ivshmem.c | 39 +++++++++++++++++++-------------------- >> 1 file changed, 19 insertions(+), 20 deletions(-) >> >> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c >> index bf585b7691998..b3983296f58fa 100644 >> --- a/hw/misc/ivshmem.c >> +++ b/hw/misc/ivshmem.c >> @@ -300,8 +300,8 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * >> chr = qemu_chr_open_eventfd(eventfd); >> >> if (chr == NULL) { >> - fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); >> - exit(-1); >> + error_report("creating eventfd for eventfd %d failed", eventfd); >> + exit(1); >> } >> qemu_chr_fe_claim_no_fail(chr); >> >> @@ -328,15 +328,14 @@ static int check_shm_size(IVShmemState *s, int fd) { >> struct stat buf; >> >> if (fstat(fd, &buf) < 0) { >> - fprintf(stderr, "ivshmem: exiting: fstat on fd %d failed: %s\n", >> + error_report("exiting: fstat on fd %d failed: %s", >> fd, strerror(errno)); > > The indentation looks weird, better to fix it.;) > More of the same elsewhere. > Er, actually, maybe for print like function, it is no need to indent like other function. So just ignore this comment, Sorry.;) >> return -1; >> } >> >> if (s->ivshmem_size > buf.st_size) { >> - fprintf(stderr, >> - "IVSHMEM ERROR: Requested memory size greater" >> - " than shared object size (%" PRIu64 " > %" PRIu64")\n", >> + error_report("Requested memory size greater" >> + " than shared object size (%" PRIu64 " > %" PRIu64")", >> s->ivshmem_size, (uint64_t)buf.st_size); >> return -1; >> } else { >> @@ -510,7 +509,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) >> incoming_fd = dup(tmp_fd); >> >> if (incoming_fd == -1) { >> - fprintf(stderr, "could not allocate file descriptor %s\n", >> + error_report("could not allocate file descriptor %s", >> strerror(errno)); >> close(tmp_fd); >> return; >> @@ -524,7 +523,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) >> s->max_peer = 0; >> >> if (check_shm_size(s, incoming_fd) == -1) { >> - exit(-1); >> + exit(1); >> } >> >> /* mmap the region and map into the BAR2 */ >> @@ -618,13 +617,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { >> value <<= 30; >> break; >> default: >> - fprintf(stderr, "qemu: invalid ram size: %s\n", s->sizearg); >> + error_report("invalid ram size: %s", s->sizearg); >> exit(1); >> } >> >> /* BARs must be a power of 2 */ >> if (!is_power_of_two(value)) { >> - fprintf(stderr, "ivshmem: size must be power of 2\n"); >> + error_report("size must be power of 2"); >> exit(1); >> } >> >> @@ -676,7 +675,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) >> } >> >> if (proxy->role_val == IVSHMEM_PEER) { >> - fprintf(stderr, "ivshmem: 'peer' devices are not migratable\n"); >> + error_report("'peer' devices are not migratable"); >> return -EINVAL; >> } >> >> @@ -722,7 +721,7 @@ static int pci_ivshmem_init(PCIDevice *dev) >> /* IRQFD requires MSI */ >> if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && >> !ivshmem_has_feature(s, IVSHMEM_MSI)) { >> - fprintf(stderr, "ivshmem: ioeventfd/irqfd requires MSI\n"); >> + error_report("ioeventfd/irqfd requires MSI"); >> exit(1); >> } >> >> @@ -733,7 +732,7 @@ static int pci_ivshmem_init(PCIDevice *dev) >> } else if (strncmp(s->role, "master", 7) == 0) { >> s->role_val = IVSHMEM_MASTER; >> } else { >> - fprintf(stderr, "ivshmem: 'role' must be 'peer' or 'master'\n"); >> + error_report("'role' must be 'peer' or 'master'"); >> exit(1); >> } >> } else { >> @@ -773,8 +772,8 @@ static int pci_ivshmem_init(PCIDevice *dev) >> * to the ivshmem server to receive the memory region */ >> >> if (s->shmobj != NULL) { >> - fprintf(stderr, "WARNING: do not specify both 'chardev' " >> - "and 'shm' with ivshmem\n"); >> + error_report("WARNING: do not specify both 'chardev' " >> + "and 'shm' with ivshmem"); >> } >> >> IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", >> @@ -802,7 +801,7 @@ static int pci_ivshmem_init(PCIDevice *dev) >> int fd; >> >> if (s->shmobj == NULL) { >> - fprintf(stderr, "Must specify 'chardev' or 'shm' to ivshmem\n"); >> + error_report("Must specify 'chardev' or 'shm' to ivshmem"); >> exit(1); >> } >> >> @@ -814,18 +813,18 @@ static int pci_ivshmem_init(PCIDevice *dev) >> S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { >> /* truncate file to length PCI device's memory */ >> if (ftruncate(fd, s->ivshmem_size) != 0) { >> - fprintf(stderr, "ivshmem: could not truncate shared file\n"); >> + error_report("could not truncate shared file"); >> } >> >> } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, >> S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { >> - fprintf(stderr, "ivshmem: could not open shared file\n"); >> - exit(-1); >> + error_report("could not open shared file"); >> + exit(1); >> >> } >> >> if (check_shm_size(s, fd) == -1) { >> - exit(-1); >> + exit(1); >> } >> >> create_shared_memory_BAR(s, fd); >> > > > > > . >
On Fri, Sep 19, 2014 at 04:42:59PM +0800, zhanghailiang wrote: > On 2014/9/19 15:34, zhanghailiang wrote: > >On 2014/9/19 7:17, Andrew Jones wrote: > >>Replace all the fprintf(stderr, ...) calls with error_report. > >>Also make sure exit() consistently uses the error code 1. A few calls > >>used -1. > >> > >>Signed-off-by: Andrew Jones <drjones@redhat.com> > >>--- > >> hw/misc/ivshmem.c | 39 +++++++++++++++++++-------------------- > >> 1 file changed, 19 insertions(+), 20 deletions(-) > >> > >>diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > >>index bf585b7691998..b3983296f58fa 100644 > >>--- a/hw/misc/ivshmem.c > >>+++ b/hw/misc/ivshmem.c > >>@@ -300,8 +300,8 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * > >> chr = qemu_chr_open_eventfd(eventfd); > >> > >> if (chr == NULL) { > >>- fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); > >>- exit(-1); > >>+ error_report("creating eventfd for eventfd %d failed", eventfd); > >>+ exit(1); > >> } > >> qemu_chr_fe_claim_no_fail(chr); > >> > >>@@ -328,15 +328,14 @@ static int check_shm_size(IVShmemState *s, int fd) { > >> struct stat buf; > >> > >> if (fstat(fd, &buf) < 0) { > >>- fprintf(stderr, "ivshmem: exiting: fstat on fd %d failed: %s\n", > >>+ error_report("exiting: fstat on fd %d failed: %s", > >> fd, strerror(errno)); > > > >The indentation looks weird, better to fix it.;) > >More of the same elsewhere. > > > > Er, actually, maybe for print like function, it is no need to indent like other function. > So just ignore this comment, Sorry.;) No, I agree with the original comment. Please indent continuation lines to the right of the opening (, here and elsewhere. > >> return -1; > >> } > >> > >> if (s->ivshmem_size > buf.st_size) { > >>- fprintf(stderr, > >>- "IVSHMEM ERROR: Requested memory size greater" > >>- " than shared object size (%" PRIu64 " > %" PRIu64")\n", > >>+ error_report("Requested memory size greater" > >>+ " than shared object size (%" PRIu64 " > %" PRIu64")", > >> s->ivshmem_size, (uint64_t)buf.st_size); > >> return -1; > >> } else { > >>@@ -510,7 +509,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > >> incoming_fd = dup(tmp_fd); > >> > >> if (incoming_fd == -1) { > >>- fprintf(stderr, "could not allocate file descriptor %s\n", > >>+ error_report("could not allocate file descriptor %s", > >> strerror(errno)); > >> close(tmp_fd); > >> return; > >>@@ -524,7 +523,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > >> s->max_peer = 0; > >> > >> if (check_shm_size(s, incoming_fd) == -1) { > >>- exit(-1); > >>+ exit(1); > >> } > >> > >> /* mmap the region and map into the BAR2 */ > >>@@ -618,13 +617,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { > >> value <<= 30; > >> break; > >> default: > >>- fprintf(stderr, "qemu: invalid ram size: %s\n", s->sizearg); > >>+ error_report("invalid ram size: %s", s->sizearg); > >> exit(1); > >> } > >> > >> /* BARs must be a power of 2 */ > >> if (!is_power_of_two(value)) { > >>- fprintf(stderr, "ivshmem: size must be power of 2\n"); > >>+ error_report("size must be power of 2"); > >> exit(1); > >> } > >> > >>@@ -676,7 +675,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) > >> } > >> > >> if (proxy->role_val == IVSHMEM_PEER) { > >>- fprintf(stderr, "ivshmem: 'peer' devices are not migratable\n"); > >>+ error_report("'peer' devices are not migratable"); > >> return -EINVAL; > >> } > >> > >>@@ -722,7 +721,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> /* IRQFD requires MSI */ > >> if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && > >> !ivshmem_has_feature(s, IVSHMEM_MSI)) { > >>- fprintf(stderr, "ivshmem: ioeventfd/irqfd requires MSI\n"); > >>+ error_report("ioeventfd/irqfd requires MSI"); > >> exit(1); > >> } > >> > >>@@ -733,7 +732,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> } else if (strncmp(s->role, "master", 7) == 0) { > >> s->role_val = IVSHMEM_MASTER; > >> } else { > >>- fprintf(stderr, "ivshmem: 'role' must be 'peer' or 'master'\n"); > >>+ error_report("'role' must be 'peer' or 'master'"); > >> exit(1); > >> } > >> } else { > >>@@ -773,8 +772,8 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> * to the ivshmem server to receive the memory region */ > >> > >> if (s->shmobj != NULL) { > >>- fprintf(stderr, "WARNING: do not specify both 'chardev' " > >>- "and 'shm' with ivshmem\n"); > >>+ error_report("WARNING: do not specify both 'chardev' " > >>+ "and 'shm' with ivshmem"); > >> } > >> > >> IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", > >>@@ -802,7 +801,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> int fd; > >> > >> if (s->shmobj == NULL) { > >>- fprintf(stderr, "Must specify 'chardev' or 'shm' to ivshmem\n"); > >>+ error_report("Must specify 'chardev' or 'shm' to ivshmem"); > >> exit(1); > >> } > >> > >>@@ -814,18 +813,18 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { > >> /* truncate file to length PCI device's memory */ > >> if (ftruncate(fd, s->ivshmem_size) != 0) { > >>- fprintf(stderr, "ivshmem: could not truncate shared file\n"); > >>+ error_report("could not truncate shared file"); > >> } > >> > >> } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, > >> S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { > >>- fprintf(stderr, "ivshmem: could not open shared file\n"); > >>- exit(-1); > >>+ error_report("could not open shared file"); > >>+ exit(1); > >> > >> } > >> > >> if (check_shm_size(s, fd) == -1) { > >>- exit(-1); > >>+ exit(1); > >> } > >> > >> create_shared_memory_BAR(s, fd); > >> > > > > > > > > > >. > > >
On Mon, Sep 22, 2014 at 02:20:09PM +0300, Michael S. Tsirkin wrote: > On Fri, Sep 19, 2014 at 04:42:59PM +0800, zhanghailiang wrote: > > On 2014/9/19 15:34, zhanghailiang wrote: > > >On 2014/9/19 7:17, Andrew Jones wrote: > > >>Replace all the fprintf(stderr, ...) calls with error_report. > > >>Also make sure exit() consistently uses the error code 1. A few calls > > >>used -1. > > >> > > >>Signed-off-by: Andrew Jones <drjones@redhat.com> > > >>--- > > >> hw/misc/ivshmem.c | 39 +++++++++++++++++++-------------------- > > >> 1 file changed, 19 insertions(+), 20 deletions(-) > > >> > > >>diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > > >>index bf585b7691998..b3983296f58fa 100644 > > >>--- a/hw/misc/ivshmem.c > > >>+++ b/hw/misc/ivshmem.c > > >>@@ -300,8 +300,8 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * > > >> chr = qemu_chr_open_eventfd(eventfd); > > >> > > >> if (chr == NULL) { > > >>- fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); > > >>- exit(-1); > > >>+ error_report("creating eventfd for eventfd %d failed", eventfd); > > >>+ exit(1); > > >> } > > >> qemu_chr_fe_claim_no_fail(chr); > > >> > > >>@@ -328,15 +328,14 @@ static int check_shm_size(IVShmemState *s, int fd) { > > >> struct stat buf; > > >> > > >> if (fstat(fd, &buf) < 0) { > > >>- fprintf(stderr, "ivshmem: exiting: fstat on fd %d failed: %s\n", > > >>+ error_report("exiting: fstat on fd %d failed: %s", > > >> fd, strerror(errno)); > > > > > >The indentation looks weird, better to fix it.;) > > >More of the same elsewhere. > > > > > > > Er, actually, maybe for print like function, it is no need to indent like other function. > > So just ignore this comment, Sorry.;) > > > No, I agree with the original comment. > Please indent continuation lines to the right of the opening (, > here and elsewhere. OK, sending a v3. drew > > > > >> return -1; > > >> } > > >> > > >> if (s->ivshmem_size > buf.st_size) { > > >>- fprintf(stderr, > > >>- "IVSHMEM ERROR: Requested memory size greater" > > >>- " than shared object size (%" PRIu64 " > %" PRIu64")\n", > > >>+ error_report("Requested memory size greater" > > >>+ " than shared object size (%" PRIu64 " > %" PRIu64")", > > >> s->ivshmem_size, (uint64_t)buf.st_size); > > >> return -1; > > >> } else { > > >>@@ -510,7 +509,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > > >> incoming_fd = dup(tmp_fd); > > >> > > >> if (incoming_fd == -1) { > > >>- fprintf(stderr, "could not allocate file descriptor %s\n", > > >>+ error_report("could not allocate file descriptor %s", > > >> strerror(errno)); > > >> close(tmp_fd); > > >> return; > > >>@@ -524,7 +523,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > > >> s->max_peer = 0; > > >> > > >> if (check_shm_size(s, incoming_fd) == -1) { > > >>- exit(-1); > > >>+ exit(1); > > >> } > > >> > > >> /* mmap the region and map into the BAR2 */ > > >>@@ -618,13 +617,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { > > >> value <<= 30; > > >> break; > > >> default: > > >>- fprintf(stderr, "qemu: invalid ram size: %s\n", s->sizearg); > > >>+ error_report("invalid ram size: %s", s->sizearg); > > >> exit(1); > > >> } > > >> > > >> /* BARs must be a power of 2 */ > > >> if (!is_power_of_two(value)) { > > >>- fprintf(stderr, "ivshmem: size must be power of 2\n"); > > >>+ error_report("size must be power of 2"); > > >> exit(1); > > >> } > > >> > > >>@@ -676,7 +675,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) > > >> } > > >> > > >> if (proxy->role_val == IVSHMEM_PEER) { > > >>- fprintf(stderr, "ivshmem: 'peer' devices are not migratable\n"); > > >>+ error_report("'peer' devices are not migratable"); > > >> return -EINVAL; > > >> } > > >> > > >>@@ -722,7 +721,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > > >> /* IRQFD requires MSI */ > > >> if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && > > >> !ivshmem_has_feature(s, IVSHMEM_MSI)) { > > >>- fprintf(stderr, "ivshmem: ioeventfd/irqfd requires MSI\n"); > > >>+ error_report("ioeventfd/irqfd requires MSI"); > > >> exit(1); > > >> } > > >> > > >>@@ -733,7 +732,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > > >> } else if (strncmp(s->role, "master", 7) == 0) { > > >> s->role_val = IVSHMEM_MASTER; > > >> } else { > > >>- fprintf(stderr, "ivshmem: 'role' must be 'peer' or 'master'\n"); > > >>+ error_report("'role' must be 'peer' or 'master'"); > > >> exit(1); > > >> } > > >> } else { > > >>@@ -773,8 +772,8 @@ static int pci_ivshmem_init(PCIDevice *dev) > > >> * to the ivshmem server to receive the memory region */ > > >> > > >> if (s->shmobj != NULL) { > > >>- fprintf(stderr, "WARNING: do not specify both 'chardev' " > > >>- "and 'shm' with ivshmem\n"); > > >>+ error_report("WARNING: do not specify both 'chardev' " > > >>+ "and 'shm' with ivshmem"); > > >> } > > >> > > >> IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", > > >>@@ -802,7 +801,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > > >> int fd; > > >> > > >> if (s->shmobj == NULL) { > > >>- fprintf(stderr, "Must specify 'chardev' or 'shm' to ivshmem\n"); > > >>+ error_report("Must specify 'chardev' or 'shm' to ivshmem"); > > >> exit(1); > > >> } > > >> > > >>@@ -814,18 +813,18 @@ static int pci_ivshmem_init(PCIDevice *dev) > > >> S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { > > >> /* truncate file to length PCI device's memory */ > > >> if (ftruncate(fd, s->ivshmem_size) != 0) { > > >>- fprintf(stderr, "ivshmem: could not truncate shared file\n"); > > >>+ error_report("could not truncate shared file"); > > >> } > > >> > > >> } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, > > >> S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { > > >>- fprintf(stderr, "ivshmem: could not open shared file\n"); > > >>- exit(-1); > > >>+ error_report("could not open shared file"); > > >>+ exit(1); > > >> > > >> } > > >> > > >> if (check_shm_size(s, fd) == -1) { > > >>- exit(-1); > > >>+ exit(1); > > >> } > > >> > > >> create_shared_memory_BAR(s, fd); > > >> > > > > > > > > > > > > > > >. > > > > > >
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index bf585b7691998..b3983296f58fa 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -300,8 +300,8 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * chr = qemu_chr_open_eventfd(eventfd); if (chr == NULL) { - fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); - exit(-1); + error_report("creating eventfd for eventfd %d failed", eventfd); + exit(1); } qemu_chr_fe_claim_no_fail(chr); @@ -328,15 +328,14 @@ static int check_shm_size(IVShmemState *s, int fd) { struct stat buf; if (fstat(fd, &buf) < 0) { - fprintf(stderr, "ivshmem: exiting: fstat on fd %d failed: %s\n", + error_report("exiting: fstat on fd %d failed: %s", fd, strerror(errno)); return -1; } if (s->ivshmem_size > buf.st_size) { - fprintf(stderr, - "IVSHMEM ERROR: Requested memory size greater" - " than shared object size (%" PRIu64 " > %" PRIu64")\n", + error_report("Requested memory size greater" + " than shared object size (%" PRIu64 " > %" PRIu64")", s->ivshmem_size, (uint64_t)buf.st_size); return -1; } else { @@ -510,7 +509,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) incoming_fd = dup(tmp_fd); if (incoming_fd == -1) { - fprintf(stderr, "could not allocate file descriptor %s\n", + error_report("could not allocate file descriptor %s", strerror(errno)); close(tmp_fd); return; @@ -524,7 +523,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) s->max_peer = 0; if (check_shm_size(s, incoming_fd) == -1) { - exit(-1); + exit(1); } /* mmap the region and map into the BAR2 */ @@ -618,13 +617,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { value <<= 30; break; default: - fprintf(stderr, "qemu: invalid ram size: %s\n", s->sizearg); + error_report("invalid ram size: %s", s->sizearg); exit(1); } /* BARs must be a power of 2 */ if (!is_power_of_two(value)) { - fprintf(stderr, "ivshmem: size must be power of 2\n"); + error_report("size must be power of 2"); exit(1); } @@ -676,7 +675,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) } if (proxy->role_val == IVSHMEM_PEER) { - fprintf(stderr, "ivshmem: 'peer' devices are not migratable\n"); + error_report("'peer' devices are not migratable"); return -EINVAL; } @@ -722,7 +721,7 @@ static int pci_ivshmem_init(PCIDevice *dev) /* IRQFD requires MSI */ if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && !ivshmem_has_feature(s, IVSHMEM_MSI)) { - fprintf(stderr, "ivshmem: ioeventfd/irqfd requires MSI\n"); + error_report("ioeventfd/irqfd requires MSI"); exit(1); } @@ -733,7 +732,7 @@ static int pci_ivshmem_init(PCIDevice *dev) } else if (strncmp(s->role, "master", 7) == 0) { s->role_val = IVSHMEM_MASTER; } else { - fprintf(stderr, "ivshmem: 'role' must be 'peer' or 'master'\n"); + error_report("'role' must be 'peer' or 'master'"); exit(1); } } else { @@ -773,8 +772,8 @@ static int pci_ivshmem_init(PCIDevice *dev) * to the ivshmem server to receive the memory region */ if (s->shmobj != NULL) { - fprintf(stderr, "WARNING: do not specify both 'chardev' " - "and 'shm' with ivshmem\n"); + error_report("WARNING: do not specify both 'chardev' " + "and 'shm' with ivshmem"); } IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", @@ -802,7 +801,7 @@ static int pci_ivshmem_init(PCIDevice *dev) int fd; if (s->shmobj == NULL) { - fprintf(stderr, "Must specify 'chardev' or 'shm' to ivshmem\n"); + error_report("Must specify 'chardev' or 'shm' to ivshmem"); exit(1); } @@ -814,18 +813,18 @@ static int pci_ivshmem_init(PCIDevice *dev) S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { /* truncate file to length PCI device's memory */ if (ftruncate(fd, s->ivshmem_size) != 0) { - fprintf(stderr, "ivshmem: could not truncate shared file\n"); + error_report("could not truncate shared file"); } } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { - fprintf(stderr, "ivshmem: could not open shared file\n"); - exit(-1); + error_report("could not open shared file"); + exit(1); } if (check_shm_size(s, fd) == -1) { - exit(-1); + exit(1); } create_shared_memory_BAR(s, fd);
Replace all the fprintf(stderr, ...) calls with error_report. Also make sure exit() consistently uses the error code 1. A few calls used -1. Signed-off-by: Andrew Jones <drjones@redhat.com> --- hw/misc/ivshmem.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)