portability patches
Subsystems (2)
cover portability patches
From: Blue Swirl <[email protected]>
I entertained myself by compiling QEMU with -std=c99 -pedantic and fixing some issues found with that.
Blue Swirl (5):
Avoid GCC extension ?:
Avoid returning void
Use __asm__ instead of asm or __asm
Avoid unportable %m format
Avoid redefining inline
block.c | 8 +++++--- block/qcow2.c | 6 ++++-- bswap.h | 40 ++++++++++++++++++++-------------------- bt-vhci.c | 2 +- cache-utils.h | 10 +++++----- dma.h | 6 +++--- exec-all.h | 6 +++--- hw/bt-hci.c | 5 +++-- hw/bt-l2cap.c | 11 +++++++---- hw/bt.c | 2 +- hw/eepro100.c | 15 +++++++++++---- hw/gumstix.c | 3 ++- hw/ide/cmd646.c | 6 +++--- hw/ide/piix.c | 3 ++- hw/ide/via.c | 3 ++- hw/lan9118.c | 6 ++++-- hw/ne2000.c | 8 ++++---- hw/omap2.c | 4 ++-- hw/omap_clk.c | 4 ++-- hw/omap_uart.c | 4 ++-- hw/onenand.c | 9 +++++---- hw/qdev-addr.c | 2 +- hw/qdev-monitor.c | 3 ++- hw/qdev-properties.c | 4 ++-- hw/qdev.c | 3 ++- hw/scsi-disk.c | 3 ++- hw/tsc210x.c | 7 +++++-- hw/usb/dev-hub.c | 2 +- hw/vmware_vga.c | 9 ++++++--- hw/wm8750.c | 9 ++++++--- hw/xen_disk.c | 4 +++- kvm-all.c | 3 ++- main-loop.c | 2 +- net/tap-linux.c | 8 +++++--- osdep.h | 2 ++ path.c | 5 ++++- qemu-barrier.h | 14 +++++++------- qemu-nbd.c | 3 +-- qemu-timer.h | 18 +++++++++--------- target-i386/cpu.c | 26 +++++++++++++------------- target-ppc/kvm.c | 4 ++-- target-ppc/kvm_ppc.h | 2 +- tcg/arm/tcg-target.h | 8 ++++---- tcg/hppa/tcg-target.h | 10 +++++----- tcg/ia64/tcg-target.h | 4 ++-- tcg/s390/tcg-target.c | 14 +++++++------- tcg/tcg.c | 2 +- tests/libqtest.c | 2 +- ui/vnc-auth-vencrypt.c | 3 ++- ui/vnc.c | 2 +- vl.c | 3 ++- 51 files changed, 193 insertions(+), 149 deletions(-)
Signature · 1 line
1.7.2.5
PATCH 1/5 Avoid GCC extension ?:
From: Blue Swirl <[email protected]>
Replace expr1 ?: expr2 with expr1 ? expr1 : expr2 as K&R intended.
If expr1 has side effects, introduce a temporary variable.
--- block.c | 6 ++++-- block/qcow2.c | 6 ++++-- bt-vhci.c | 2 +- hw/bt-hci.c | 5 +++-- hw/bt.c | 2 +- hw/gumstix.c | 3 ++- hw/omap2.c | 4 ++-- hw/omap_clk.c | 4 ++-- hw/omap_uart.c | 4 ++-- hw/onenand.c | 9 +++++---- hw/qdev-addr.c | 2 +- hw/qdev-monitor.c | 3 ++- hw/qdev-properties.c | 4 ++-- hw/qdev.c | 3 ++- hw/scsi-disk.c | 3 ++- hw/tsc210x.c | 7 +++++-- hw/usb/dev-hub.c | 2 +- hw/wm8750.c | 9 ++++++--- hw/xen_disk.c | 4 +++- path.c | 5 ++++- tests/libqtest.c | 2 +- vl.c | 3 ++- 22 files changed, 57 insertions(+), 35 deletions(-)
block.c +4 -2
@@ -1499,8 +1499,10 @@ int bdrv_change_backing_file(BlockDriverState *bs, }if (ret ==0 ) { -pstrcpy (bs->backing_file,sizeof (bs->backing_file), backing_file ?:"" ); -pstrcpy (bs->backing_format,sizeof (bs->backing_format), backing_fmt ?:"" ); +pstrcpy (bs->backing_file,sizeof (bs->backing_file), backing_file ? + backing_file :"" ); +pstrcpy (bs->backing_format,sizeof (bs->backing_format), backing_fmt ? + backing_fmt :"" ); }return ret; }
block/qcow2.c +4 -2
@@ -1013,8 +1013,10 @@ fail:static int qcow2_change_backing_file (BlockDriverState *bs,const char *backing_file,const char *backing_fmt) { -pstrcpy (bs->backing_file,sizeof (bs->backing_file), backing_file ?:"" ); -pstrcpy (bs->backing_format,sizeof (bs->backing_format), backing_fmt ?:"" ); +pstrcpy (bs->backing_file,sizeof (bs->backing_file), + backing_file ? backing_file :"" ); +pstrcpy (bs->backing_format,sizeof (bs->backing_format), + backing_fmt ? backing_file :"" );return qcow2_update_header (bs); }
bt-vhci.c +1 -1
@@ -158,7 +158,7 @@ void bt_vhci_init(struct HCIInfo *info) s =g_malloc0 (sizeof (struct bt_vhci_s )); s->fd = fd; - s->info = info ?:qemu_next_hci (); + s->info = info ? info :qemu_next_hci (); s->info->opaque = s; s->info->evt_recv = vhci_out_hci_packet_event; s->info->acl_recv = vhci_out_hci_packet_acl;
hw/bt-hci.c +3 -2
@@ -956,7 +956,7 @@ static int bt_hci_name_req(struct bt_hci_s *hci, bdaddr_t *bdaddr) params.status =HCI_SUCCESS ;bacpy (¶ms.bdaddr, &slave->bd_addr); len =snprintf (params.name,sizeof (params.name), -"%s" , slave->lmp_name ?:"" ); +"%s" , slave->lmp_name ? slave->lmp_name :"" );memset (params.name + len,0 ,sizeof (params.name) - len);bt_hci_event (hci,EVT_REMOTE_NAME_REQ_COMPLETE , ¶ms,EVT_REMOTE_NAME_REQ_COMPLETE_SIZE );
@@ -1500,7 +1500,8 @@ static void bt_submit_hci(struct HCIInfo *info, hci->lm.inquire =1 ; hci->lm.periodic =0 ; - hci->lm.responses_left =PARAM (inquiry, num_rsp) ?:INT_MAX ; + hci->lm.responses_left =PARAM (inquiry, num_rsp) ? +PARAM (inquiry, num_rsp) :INT_MAX ; hci->lm.responses =0 ;bt_hci_event_status (hci,HCI_SUCCESS );bt_hci_inquiry_start (hci,PARAM (inquiry, length));
hw/bt.c +1 -1
@@ -113,7 +113,7 @@ void bt_device_done(struct bt_device_s *dev) p = &(*p)->next;if (*p != dev) {fprintf (stderr,"%s: bad bt device \"%s\"\n" , __FUNCTION__, - dev->lmp_name ?:"(null)" ); + dev->lmp_name ? dev->lmp_name :"(null)" );exit (-1 ); }
hw/gumstix.c +2 -1
@@ -97,7 +97,8 @@ static void verdex_init(ram_addr_t ram_size,uint32_t verdex_rom =0x02000000 ;uint32_t verdex_ram =0x10000000 ; - cpu =pxa270_init (address_space_mem, verdex_ram, cpu_model ?:"pxa270-c0" ); + cpu =pxa270_init (address_space_mem, verdex_ram, + cpu_model ? cpu_model :"pxa270-c0" ); dinfo =drive_get (IF_PFLASH ,0 ,0 );if (!dinfo) {
hw/omap2.c +2 -2
@@ -789,7 +789,7 @@ static struct omap_sti_s *omap_sti_init(struct omap_target_agent_s *ta, s->irq = irq;omap_sti_reset (s); - s->chr = chr ?:qemu_chr_new ("null" ,"null" ,NULL ); + s->chr = chr ? chr :qemu_chr_new ("null" ,"null" ,NULL );memory_region_init_io (&s->iomem, &omap_sti_ops, s,"omap.sti" ,omap_l4_region_size (ta,0 ));
@@ -2253,7 +2253,7 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem,/* Core */ s->mpu_model = omap2420; - s->cpu =cpu_arm_init (core ?:"arm1136-r2" ); + s->cpu =cpu_arm_init (core ? core :"arm1136-r2" );if (s->cpu ==NULL ) {fprintf (stderr,"Unable to find CPU definition\n" );exit (1 );
hw/omap_clk.c +2 -2
@@ -1253,8 +1253,8 @@ void omap_clk_init(struct omap_mpu_state_s *mpu) k->sibling = j->child1; j->child1 = k; } - j->divisor = j->divisor ?:1 ; - j->multiplier = j->multiplier ?:1 ; + j->divisor = j->divisor ? j->divisor :1 ; + j->multiplier = j->multiplier ? j->multiplier :1 ; j ++; }for (j = mpu->clks; count --; j ++) {
hw/omap_uart.c +2 -2
@@ -64,7 +64,7 @@ struct omap_uart_s *omap_uart_init(target_phys_addr_t base, s->irq = irq; s->serial =serial_mm_init (get_system_memory (), base,2 , irq,omap_clk_getrate (fclk)/16 , - chr ?:qemu_chr_new (label,"null" ,NULL ), + chr ? chr :qemu_chr_new (label,"null" ,NULL ),DEVICE_NATIVE_ENDIAN );return s; }
@@ -183,6 +183,6 @@ void omap_uart_attach(struct omap_uart_s *s, CharDriverState *chr)/* TODO: Should reuse or destroy current s->serial */ s->serial =serial_mm_init (get_system_memory (), s->base,2 , s->irq,omap_clk_getrate (s->fclk) /16 , - chr ?:qemu_chr_new ("null" ,"null" ,NULL ), + chr ? chr :qemu_chr_new ("null" ,"null" ,NULL ),DEVICE_NATIVE_ENDIAN ); }
hw/onenand.c +5 -4
@@ -704,10 +704,11 @@ static void onenand_write(void *opaque, target_phys_addr_t addr,case 0xf200 :/* Start buffer */ s->bufaddr = (value >>8 ) &0xf ; -if (PAGE_SHIFT ==11 ) - s->count = (value &3 ) ?:4 ; -else if (PAGE_SHIFT ==10 ) - s->count = (value &1 ) ?:2 ; +if (PAGE_SHIFT ==11 ) { + s->count = (value &3 ) ? (value &3 ) :4 ; + }else if (PAGE_SHIFT ==10 ) { + s->count = (value &1 ) ? (value &1 ) :2 ; + }break ;case 0xf220 :/* Command */
hw/qdev-addr.c +1 -1
@@ -53,7 +53,7 @@ static void set_taddr(Object *obj, Visitor *v, void *opaque, *ptr = value; }else {error_set (errp,QERR_PROPERTY_VALUE_OUT_OF_RANGE , - dev->id?:"" , name, value, (uint64_t )0 , + dev->id ? dev->id :"" , name, value, (uint64_t )0 , (uint64_t ) ~(target_phys_addr_t )0 ); } }
hw/qdev-monitor.c +2 -1
@@ -173,7 +173,8 @@ int qdev_device_help(QemuOpts *opts)continue ;/* no way to set it, don't show */ }error_printf ("%s.%s=%s\n" , driver, prop->name, - prop->info->legacy_name ?: prop->info->name); + prop->info->legacy_name ? prop->info->legacy_name : + prop->info->name); } klass =object_class_get_parent (klass); }while (klass !=object_class_by_name (TYPE_DEVICE ));
hw/qdev-properties.c +2 -2
@@ -879,14 +879,14 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, }if (value < min || value > max) {error_set (errp,QERR_PROPERTY_VALUE_OUT_OF_RANGE , - dev->id?:"" , name, (int64_t )value, min, max); + dev->id ? dev->id :"" , name, (int64_t )value, min, max);return ; }/* We rely on power-of-2 blocksizes for bitmasks */ if ((value & (value -1 )) !=0 ) {error_set (errp,QERR_PROPERTY_VALUE_NOT_POWER_OF_2 , - dev->id?:"" , name, (int64_t )value); + dev->id ? dev->id :"" , name, (int64_t )value);return ; }
hw/qdev.c +2 -1
@@ -607,7 +607,8 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop, name =g_strdup_printf ("legacy-%s" , prop->name); type =g_strdup_printf ("legacy<%s>" , - prop->info->legacy_name ?: prop->info->name); + prop->info->legacy_name ? prop->info->legacy_name : + prop->info->name); object_property_add(OBJECT(dev), name, type, prop->info->print ? qdev_get_legacy_property : prop->info->get,
hw/scsi-disk.c +2 -1
@@ -574,7 +574,8 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)case 0x83 :/* Device identification page, mandatory */ { -const char *str = s->serial ?:bdrv_get_device_name (s->qdev.conf.bs); +const char *str = s->serial ? s->serial : +bdrv_get_device_name (s->qdev.conf.bs);int max_len = s->serial ?20 :255 -8 ;int id_len =strlen (str);
hw/tsc210x.c +5 -2
@@ -279,9 +279,12 @@ static inline void tsc210x_out_flush(TSC210xState *s, int len) {uint8_t *data = s->codec.out.fifo + s->codec.out.start;uint8_t *end = data + len; +int written; -while (data < end) - data +=AUD_write (s->dac_voice[0 ], data, end - data) ?: (end - data); +while (data < end) { + written =AUD_write (s->dac_voice[0 ], data, end - data); + data += written ? written : (end - data); + } s->codec.out.len -= len;if (s->codec.out.len)
hw/usb/dev-hub.c +1 -1
@@ -285,7 +285,7 @@ static const char *feature_name(int feature)if (feature <0 || feature >=ARRAY_SIZE (name)) {return "?" ; } -return name[feature] ?:"?" ; +return name[feature] ? name[feature] :"?" ; } static int usb_hub_handle_control(USBDevice *dev, USBPacket *p,
hw/wm8750.c +6 -3
@@ -72,9 +72,12 @@ static inline void wm8750_in_load(WM8750State *s)static inline void wm8750_out_flush (WM8750State *s) {int sent =0 ; -while (sent < s->idx_out) - sent +=AUD_write (*s->out[0 ], s->data_out + sent, s->idx_out - sent) - ?: s->idx_out; +int written; + +while (sent < s->idx_out) { + written =AUD_write (*s->out[0 ], s->data_out + sent, s->idx_out - sent); + sent += written ? written : s->idx_out; + } s->idx_out =0 ; }
hw/xen_disk.c +3 -1
@@ -643,9 +643,11 @@ static int blk_init(struct XenDevice *xendev) blkdev->file_blk =BLOCK_SIZE ; blkdev->file_size =bdrv_getlength (blkdev->bs);if (blkdev->file_size <0 ) { +char *name =bdrv_get_format_name (blkdev->bs); +xen_be_printf (&blkdev->xendev,1 ,"bdrv_getlength: %d (%s) | drv %s\n" , (int )blkdev->file_size,strerror (-blkdev->file_size), -bdrv_get_format_name (blkdev->bs) ?:"-" ); + name ? name :"-" ); blkdev->file_size =0 ; }
path.c +4 -1
@@ -172,10 +172,13 @@ void init_paths(const char *prefix)/* Look for path in emulation dir, otherwise return name. */ const char *path (const char *name) { +const char *ret; +/* Only do absolute paths: quick and dirty, but should mostly be OK. Could do relative by tracking cwd. */ if (!base || !name || name[0 ] !='/' )return name; -return follow_path (base, name) ?: name; + ret =follow_path (base, name); +return ret ? ret : name; }
tests/libqtest.c +1 -1
@@ -117,7 +117,7 @@ QTestState *qtest_init(const char *extra_args)"-machine accel=qtest " "%s" , qemu_binary, socket_path, qmp_socket_path, pid_file, - extra_args ?: ""); + extra_args ? extra_args : ""); ret =system (command);exit (ret);
vl.c +2 -1
@@ -3260,7 +3260,8 @@ int main(int argc, char **argv, char **envp)if (!max_cpus) max_cpus = smp_cpus; - machine->max_cpus = machine->max_cpus ?:1 ;/* Default to UP */ +/* Default to UP */ + machine->max_cpus = machine->max_cpus ? machine->max_cpus :1 ;if (smp_cpus > machine->max_cpus) { fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus " "supported by machine `%s' (%d)\n", smp_cpus, machine->name,
Signature · 1 line
1.7.2.5
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
block/qcow2.c fail: · line 1019
Quoted 2 lines · [email protected] writes:
block/qcow2.c +2
-0+pstrcpy (bs->backing_format,sizeof (bs->backing_format), + backing_fmt ? backing_file :"" );
s/backing_file/backing_fmt/
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Quoted 6 lines · Andreas Schwab
block/qcow2.c fail: · line 1019
Quoted 2 lines · [email protected] writes:
block/qcow2.c +2
-0+pstrcpy (bs->backing_format,sizeof (bs->backing_format), + backing_fmt ? backing_file :"" );s/backing_file/backing_fmt/
Thanks, will fix.
I accidentally sent the patches using wrong address.
Quoted 7 lines
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
In reply to Andreas SchwabAm 08.07.2012 14:09, schrieb Andreas Schwab:
Quoted 6 lines
block/qcow2.c fail: · line 1019
Quoted 2 lines · [email protected] writes:
block/qcow2.c +2
-0+pstrcpy (bs->backing_format,sizeof (bs->backing_format), + backing_fmt ? backing_file :"" );s/backing_file/backing_fmt/
Which is why such changes are probably a bad idea. Even more so if they aren't scripted.
Does this patch improve anything? Last time I checked, qemu only compiled on gcc anyway.
Kevin
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Quoted 2 lines · Kevin Wolf
Does this patch improve anything? Last time I checked, qemu only compiled on gcc anyway.
It would be nice to be able to compile with LLVM/Clang; however since Clang supports the ?: gcc extension this patch doesn't move us any further in that direction.
-- PMM
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Quoted 7 lines · Peter Maydell
Quoted 2 lines · Kevin Wolf
Does this patch improve anything? Last time I checked, qemu only compiled on gcc anyway.
It would be nice to be able to compile with LLVM/Clang; however since Clang supports the ?: gcc extension this patch doesn't move us any further in that direction.
Let's get AVOID_PASS_ARGV0 patch done. ;)
Regards,
chenwj
Signature · 5 lines
Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Quoted 10 lines · 陳韋任 (Wei-Ren Chen)
Quoted 7 lines · Peter Maydell
Quoted 2 lines · Kevin Wolf
Does this patch improve anything? Last time I checked, qemu only compiled on gcc anyway.
It would be nice to be able to compile with LLVM/Clang; however since Clang supports the ?: gcc extension this patch doesn't move us any further in that direction.
Let's get
AVOID_PASS_ARGV0patch done. ;)
Status report: I've narrowed the bug in the x86 AREG0 patches to SSE conversion. Converting all x86 code except most SSE functions and the final step (blocked by SSE) is still OK.
Quoted 10 lines
Regards,
chenwj--
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
In reply to Kevin WolfQuoted 10 lines · Kevin Wolf
Am 08.07.2012 14:09, schrieb Andreas Schwab:
Quoted 6 lines
block/qcow2.c fail: · line 1019
Quoted 2 lines · [email protected] writes:
block/qcow2.c +2
-0+pstrcpy (bs->backing_format,sizeof (bs->backing_format), + backing_fmt ? backing_file :"" );s/backing_file/backing_fmt/
Which is why such changes are probably a bad idea. Even more so if they aren't scripted.
Maybe your patches are perfect from day one, but all patches can be buggy. Review should catch some of the bugs, others may be found later. It's not possible to script this because expr1 may have side effects.
Quoted 3 lines
Does this patch improve anything? Last time I checked, qemu only compiled on gcc anyway.
It improves C99 compliance. GCC extensions should not be used unless absolutely required. In the future, it should be possible to compile QEMU with any C compiler, AREG0 patches remove the biggest obstacle.
Quoted 2 lines
Kevin
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Quoted 7 lines · Blue Swirl
Quoted 3 lines · Kevin Wolf
Am 08.07.2012 14:09, schrieb Andreas Schwab:
Which is why such changes are probably a bad idea. Even more so if they aren't scripted.Maybe your patches are perfect from day one, but all patches can be buggy.
It's exactly *because* all patches can be buggy that changes need to demonstrate a clear benefit in order that the expected gain overall from applying the patch is positive rather than negative.
-- PMM
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Quoted 6 lines · Peter Maydell
Quoted 5 lines · Blue Swirl
Quoted 3 lines · Kevin Wolf
Am 08.07.2012 14:09, schrieb Andreas Schwab:
Which is why such changes are probably a bad idea. Even more so if they aren't scripted.
Quoted 6 lines
Quoted 2 lines
Maybe your patches are perfect from day one, but all patches can be buggy.
It's exactly *because* all patches can be buggy that changes need to demonstrate a clear benefit in order that the expected gain overall from applying the patch is positive rather than negative.
Fine: there is the demonstrated clear benefit of improved standards compliance.
Quoted 2 lines
-- PMM
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
In reply to Blue SwirlAm 12.07.2012 22:28, schrieb Blue Swirl:
Quoted 9 lines · Kevin Wolf
Quoted 8 lines
Am 08.07.2012 14:09, schrieb Andreas Schwab:
Quoted 6 lines
block/qcow2.c fail: · line 1019
Quoted 2 lines · [email protected] writes:
block/qcow2.c +2
-0+pstrcpy (bs->backing_format,sizeof (bs->backing_format), + backing_fmt ? backing_file :"" );s/backing_file/backing_fmt/
Quoted 7 lines
Quoted 2 lines
Which is why such changes are probably a bad idea. Even more so if they aren't scripted.
Maybe your patches are perfect from day one, but all patches can be buggy. Review should catch some of the bugs, others may be found later. It's not possible to script this because expr1 may have side effects.
No, my patches aren't perfect, each patch is a risk. So all I'm saying is that if it ain't broke, don't fix it.
Quoted 6 lines
Quoted 2 lines
Does this patch improve anything? Last time I checked, qemu only compiled on gcc anyway.
It improves C99 compliance. GCC extensions should not be used unless absolutely required. In the future, it should be possible to compile QEMU with any C compiler, AREG0 patches remove the biggest obstacle.
If this is our goal and we're really close, it might be worth these changes. Are you working towards getting a specific compiler to build qemu? Can we get a buildbot for this compiler once it works for the first time? Because otherwise I'm pretty sure that it will break frequently.
Kevin
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Quoted 13 lines · Kevin Wolf
Am 12.07.2012 22:28, schrieb Blue Swirl:
Quoted 12 lines · Kevin Wolf
Quoted 10 lines
Am 08.07.2012 14:09, schrieb Andreas Schwab:
Quoted 6 lines
block/qcow2.c fail: · line 1019
Quoted 2 lines · [email protected] writes:
block/qcow2.c +2
-0+pstrcpy (bs->backing_format,sizeof (bs->backing_format), + backing_fmt ? backing_file :"" );s/backing_file/backing_fmt/
Which is why such changes are probably a bad idea. Even more so if they aren't scripted.
Quoted 7 lines
Quoted 4 lines
Maybe your patches are perfect from day one, but all patches can be buggy. Review should catch some of the bugs, others may be found later. It's not possible to script this because expr1 may have side effects.
No, my patches aren't perfect, each patch is a risk. So all I'm saying is that if it ain't broke, don't fix it.
That way leads to stagnated code. If a change is useful and matches overall architecture, it should be applied.
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Does this patch improve anything? Last time I checked, qemu only compiled on gcc anyway.
Quoted 8 lines
Quoted 3 lines
It improves C99 compliance. GCC extensions should not be used unless absolutely required. In the future, it should be possible to compile QEMU with any C compiler, AREG0 patches remove the biggest obstacle.
If this is our goal and we're really close, it might be worth these changes. Are you working towards getting a specific compiler to build qemu? Can we get a buildbot for this compiler once it works for the first time? Because otherwise I'm pretty sure that it will break frequently.
Is it so hard to avoid GCCisms? Perhaps checkpatch.pl (which seems to be ignored by many people) could be improved to detect this.
I found these with GCC flag -std=c99. There were plenty of other errors which may or may not be worth fixing. Setting up a buildbot with this flag should be possible.
Quoted 2 lines
Kevin
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
In reply to [email protected]Quoted 23 lines · [email protected] writes:
From: Blue Swirl <[email protected]>
Replace expr1 ?: expr2 with expr1 ? expr1 : expr2 as K&R intended.
If expr1 has side effects, introduce a temporary variable.
Signed-off-by Blue Swirl <[email protected]>--- block.c | 6 ++++-- block/qcow2.c | 6 ++++-- bt-vhci.c | 2 +- hw/bt-hci.c | 5 +++-- hw/bt.c | 2 +- hw/gumstix.c | 3 ++- hw/omap2.c | 4 ++-- hw/omap_clk.c | 4 ++-- hw/omap_uart.c | 4 ++-- hw/onenand.c | 9 +++++---- hw/qdev-addr.c | 2 +- hw/qdev-monitor.c | 3 ++- hw/qdev-properties.c | 4 ++-- hw/qdev.c | 3 ++- hw/scsi-disk.c | 3 ++-
Quoted 8 lines
hw/tsc210x.c | 7 +++++-- hw/usb/dev-hub.c | 2 +- hw/wm8750.c | 9 ++++++--- hw/xen_disk.c | 4 +++- path.c | 5 ++++- tests/libqtest.c | 2 +- vl.c | 3 ++- 22 files changed, 57 insertions(+), 35 deletions(-)
Practical benefits?
Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Quoted 34 lines · Markus Armbruster
Quoted 23 lines · [email protected] writes:
From: Blue Swirl <[email protected]>
Replace expr1 ?: expr2 with expr1 ? expr1 : expr2 as K&R intended.
If expr1 has side effects, introduce a temporary variable.
Signed-off-by Blue Swirl <[email protected]>--- block.c | 6 ++++-- block/qcow2.c | 6 ++++-- bt-vhci.c | 2 +- hw/bt-hci.c | 5 +++-- hw/bt.c | 2 +- hw/gumstix.c | 3 ++- hw/omap2.c | 4 ++-- hw/omap_clk.c | 4 ++-- hw/omap_uart.c | 4 ++-- hw/onenand.c | 9 +++++---- hw/qdev-addr.c | 2 +- hw/qdev-monitor.c | 3 ++- hw/qdev-properties.c | 4 ++-- hw/qdev.c | 3 ++- hw/scsi-disk.c | 3 ++-Quoted 8 lines
hw/tsc210x.c | 7 +++++-- hw/usb/dev-hub.c | 2 +- hw/wm8750.c | 9 ++++++--- hw/xen_disk.c | 4 +++- path.c | 5 ++++- tests/libqtest.c | 2 +- vl.c | 3 ++- 22 files changed, 57 insertions(+), 35 deletions(-)
Quoted 1 line
Practical benefits?
Improved C99 compliance. ?: isn't in C11 either, unlike for example anonymous unions.
PATCH 2/5 Avoid returning void
In reply to [email protected]From: Blue Swirl <[email protected]>
It's silly and non-conforming to standards to return void, don't do it.
--- block.c | 2 +- dma.h | 6 +++--- hw/bt-l2cap.c | 11 +++++++---- hw/eepro100.c | 15 +++++++++++---- hw/ide/cmd646.c | 6 +++--- hw/ide/piix.c | 3 ++- hw/ide/via.c | 3 ++- hw/lan9118.c | 6 ++++-- hw/ne2000.c | 8 ++++---- hw/vmware_vga.c | 9 ++++++--- ui/vnc-auth-vencrypt.c | 3 ++- ui/vnc.c | 2 +- 12 files changed, 46 insertions(+), 28 deletions(-)
block.c +1 -1
@@ -2832,7 +2832,7 @@ void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event)return ; } -return drv->bdrv_debug_event (bs, event); + drv->bdrv_debug_event (bs, event); }
dma.h +3 -3
@@ -196,9 +196,9 @@ static inline void dma_memory_unmap(DMAContext *dma, DMADirection dir, dma_addr_t access_len) {if (!dma_has_iommu (dma)) { -return cpu_physical_memory_unmap (buffer, (target_phys_addr_t )len, - dir ==DMA_DIRECTION_FROM_DEVICE , - access_len); +cpu_physical_memory_unmap (buffer, (target_phys_addr_t )len, + dir ==DMA_DIRECTION_FROM_DEVICE , + access_len); }else {iommu_dma_memory_unmap (dma, buffer, len, dir, access_len); }
hw/bt-l2cap.c +7 -4
@@ -1000,7 +1000,8 @@ static void l2cap_iframe_in(struct l2cap_chan_s *ch, uint16_t cid,/* TODO: Signal an error? */ return ; } -return l2cap_sframe_in (ch,le16_to_cpup ((void *) hdr->data)); +l2cap_sframe_in (ch,le16_to_cpup ((void *) hdr->data)); +return ; }switch (hdr->data[1 ] >>6 ) {/* SAR */
@@ -1010,7 +1011,8 @@ static void l2cap_iframe_in(struct l2cap_chan_s *ch, uint16_t cid,if (len -4 > ch->mps) goto len_error; -return ch->params.sdu_in (ch->params.opaque, hdr->data +2 , len -4 ); + ch->params.sdu_in (ch->params.opaque, hdr->data +2 , len -4 ); +break ;case L2CAP_SAR_START :if (ch->len_total || len <6 )
@@ -1033,7 +1035,8 @@ static void l2cap_iframe_in(struct l2cap_chan_s *ch, uint16_t cid, goto len_error;memcpy (ch->sdu + ch->len_cur, hdr->data +2 , len -4 ); -return ch->params.sdu_in (ch->params.opaque, ch->sdu, ch->len_total); + ch->params.sdu_in (ch->params.opaque, ch->sdu, ch->len_total); +break ;case L2CAP_SAR_CONT :if (!ch->len_total || ch->len_cur + len -4 >= ch->len_total)
@@ -1136,7 +1139,7 @@ static void l2cap_bframe_submit(struct bt_l2cap_conn_params_s *parms) {struct l2cap_chan_s *chan = (struct l2cap_chan_s *) parms; -return l2cap_pdu_submit (chan->l2cap); +l2cap_pdu_submit (chan->l2cap); }#if 0
hw/eepro100.c +11 -4
@@ -1596,10 +1596,17 @@ static void eepro100_write(void *opaque, target_phys_addr_t addr,EEPRO100State *s = opaque;switch (size) { -case 1 :return eepro100_write1 (s, addr, data); -case 2 :return eepro100_write2 (s, addr, data); -case 4 :return eepro100_write4 (s, addr, data); -default :abort (); +case 1 : +eepro100_write1 (s, addr, data); +break ; +case 2 : +eepro100_write2 (s, addr, data); +break ; +case 4 : +eepro100_write4 (s, addr, data); +break ; +default : +abort (); } }
hw/ide/cmd646.c +3 -3
@@ -94,12 +94,12 @@ static void cmd646_data_write(void *opaque, target_phys_addr_t addr,CMD646BAR *cmd646bar = opaque;if (size ==1 ) { -return ide_ioport_write (cmd646bar->bus, addr, data); +ide_ioport_write (cmd646bar->bus, addr, data); }else if (addr ==0 ) {if (size ==2 ) { -return ide_data_writew (cmd646bar->bus, addr, data); +ide_data_writew (cmd646bar->bus, addr, data); }else { -return ide_data_writel (cmd646bar->bus, addr, data); +ide_data_writel (cmd646bar->bus, addr, data); } } }
hw/ide/piix.c +2 -1
@@ -73,7 +73,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,#endif switch (addr &3 ) {case 0 : -return bmdma_cmd_writeb (bm, val); +bmdma_cmd_writeb (bm, val); +break ;case 2 : bm->status = (val &0x60 ) | (bm->status &1 ) | (bm->status & ~val &0x06 );break ;
hw/ide/via.c +2 -1
@@ -74,7 +74,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,#endif switch (addr &3 ) {case 0 : -return bmdma_cmd_writeb (bm, val); +bmdma_cmd_writeb (bm, val); +break ;case 2 : bm->status = (val &0x60 ) | (bm->status &1 ) | (bm->status & ~val &0x06 );break ;
hw/lan9118.c +4 -2
@@ -1166,9 +1166,11 @@ static void lan9118_16bit_mode_write(void *opaque, target_phys_addr_t offset, {switch (size) {case 2 : -return lan9118_writew (opaque, offset, (uint32_t )val); +lan9118_writew (opaque, offset, (uint32_t )val); +return ;case 4 : -return lan9118_writel (opaque, offset, val, size); +lan9118_writel (opaque, offset, val, size); +return ; }hw_error ("lan9118_write: Bad size 0x%x\n" , size);
hw/ne2000.c +4 -4
@@ -677,15 +677,15 @@ static void ne2000_write(void *opaque, target_phys_addr_t addr,NE2000State *s = opaque;if (addr <0x10 && size ==1 ) { -return ne2000_ioport_write (s, addr, data); +ne2000_ioport_write (s, addr, data); }else if (addr ==0x10 ) {if (size <=2 ) { -return ne2000_asic_ioport_write (s, addr, data); +ne2000_asic_ioport_write (s, addr, data); }else { -return ne2000_asic_ioport_writel (s, addr, data); +ne2000_asic_ioport_writel (s, addr, data); } }else if (addr ==0x1f && size ==1 ) { -return ne2000_reset_ioport_write (s, addr, data); +ne2000_reset_ioport_write (s, addr, data); } }
hw/vmware_vga.c +6 -3
@@ -1150,11 +1150,14 @@ static void vmsvga_io_write(void *opaque, target_phys_addr_t addr,switch (addr) {case SVGA_IO_MUL *SVGA_INDEX_PORT : -return vmsvga_index_write (s, addr, data); +vmsvga_index_write (s, addr, data); +break ;case SVGA_IO_MUL *SVGA_VALUE_PORT : -return vmsvga_value_write (s, addr, data); +vmsvga_value_write (s, addr, data); +break ;case SVGA_IO_MUL *SVGA_BIOS_PORT : -return vmsvga_bios_write (s, addr, data); +vmsvga_bios_write (s, addr, data); +break ; } }
ui/vnc-auth-vencrypt.c +2 -1
@@ -47,7 +47,8 @@ static void start_auth_vencrypt_subauth(VncState *vs)case VNC_AUTH_VENCRYPT_TLSSASL :case VNC_AUTH_VENCRYPT_X509SASL :VNC_DEBUG ("Start TLS auth SASL\n" ); -return start_auth_sasl (vs); +start_auth_sasl (vs); +break ;#endif /* CONFIG_VNC_SASL */ default :/* Should not be possible, but just in case */
ui/vnc.c +1 -1
@@ -3089,5 +3089,5 @@ void vnc_display_add_client(DisplayState *ds, int csock, int skipauth) {VncDisplay *vs = ds ? (VncDisplay *)ds->opaque : vnc_display; -return vnc_connect (vs, csock, skipauth); +vnc_connect (vs, csock, skipauth); }
Signature · 1 line
1.7.2.5
PATCH 3/5 Use __asm__ instead of asm or __asm
In reply to [email protected]From: Blue Swirl <[email protected]>
Replace asm and __asm with __asm__.
--- bswap.h | 40 ++++++++++++++++++++-------------------- cache-utils.h | 10 +++++----- exec-all.h | 6 +++--- qemu-barrier.h | 14 +++++++------- qemu-timer.h | 18 +++++++++--------- target-i386/cpu.c | 26 +++++++++++++------------- target-ppc/kvm.c | 4 ++-- target-ppc/kvm_ppc.h | 2 +- tcg/arm/tcg-target.h | 8 ++++---- tcg/hppa/tcg-target.h | 10 +++++----- tcg/ia64/tcg-target.h | 4 ++-- tcg/s390/tcg-target.c | 14 +++++++------- tcg/tcg.c | 2 +- 13 files changed, 79 insertions(+), 79 deletions(-)
bswap.h +20 -20
@@ -526,10 +526,10 @@ static inline int lduw_be_p(const void *ptr) {#if defined(__i386__)int val; - asmvolatile ("movzwl %1, %0\n" -"xchgb %b0, %h0\n" - :"=q" (val) - :"m" (*(uint16_t *)ptr)); + __asm__volatile ("movzwl %1, %0\n" +"xchgb %b0, %h0\n" + :"=q" (val) + :"m" (*(uint16_t *)ptr));return val;#else const uint8_t *b = ptr;
@@ -541,10 +541,10 @@ static inline int ldsw_be_p(const void *ptr) {#if defined(__i386__)int val; - asmvolatile ("movzwl %1, %0\n" -"xchgb %b0, %h0\n" - :"=q" (val) - :"m" (*(uint16_t *)ptr)); + __asm__volatile ("movzwl %1, %0\n" +"xchgb %b0, %h0\n" + :"=q" (val) + :"m" (*(uint16_t *)ptr));return (int16_t )val;#else const uint8_t *b = ptr;
@@ -556,10 +556,10 @@ static inline int ldl_be_p(const void *ptr) {#if defined(__i386__) || defined(__x86_64__)int val; - asmvolatile ("movl %1, %0\n" -"bswap %0\n" - :"=r" (val) - :"m" (*(uint32_t *)ptr)); + __asm__volatile ("movl %1, %0\n" +"bswap %0\n" + :"=r" (val) + :"m" (*(uint32_t *)ptr));return val;#else const uint8_t *b = ptr;
@@ -578,10 +578,10 @@ static inline uint64_t ldq_be_p(const void *ptr)static inline void stw_be_p (void *ptr,int v) {#if defined(__i386__) - asmvolatile ("xchgb %b0, %h0\n" -"movw %w0, %1\n" - :"=q" (v) - :"m" (*(uint16_t *)ptr),"0" (v)); + __asm__volatile ("xchgb %b0, %h0\n" +"movw %w0, %1\n" + :"=q" (v) + :"m" (*(uint16_t *)ptr),"0" (v));#else uint8_t *d = (uint8_t *) ptr; d[0 ] = v >>8 ;
@@ -592,10 +592,10 @@ static inline void stw_be_p(void *ptr, int v)static inline void stl_be_p (void *ptr,int v) {#if defined(__i386__) || defined(__x86_64__) - asmvolatile ("bswap %0\n" -"movl %0, %1\n" - :"=r" (v) - :"m" (*(uint32_t *)ptr),"0" (v)); + __asm__volatile ("bswap %0\n" +"movl %0, %1\n" + :"=r" (v) + :"m" (*(uint32_t *)ptr),"0" (v));#else uint8_t *d = (uint8_t *) ptr; d[0 ] = v >>24 ;
cache-utils.h +5 -5
@@ -24,17 +24,17 @@ static inline void flush_icache_range(uintptr_t start, uintptr_t stop) start1 = start & ~(dsize -1 ); stop1 = (stop + dsize -1 ) & ~(dsize -1 );for (p = start1; p < stop1; p += dsize) { - asmvolatile ("dcbst 0,%0" : :"r" (p) :"memory" ); + __asm__volatile ("dcbst 0,%0" : :"r" (p) :"memory" ); } - asmvolatile ("sync" : : :"memory" ); + __asm__volatile ("sync" : : :"memory" ); start &= start & ~(isize -1 ); stop1 = (stop + isize -1 ) & ~(isize -1 );for (p = start1; p < stop1; p += isize) { - asmvolatile ("icbi 0,%0" : :"r" (p) :"memory" ); + __asm__volatile ("icbi 0,%0" : :"r" (p) :"memory" ); } - asmvolatile ("sync" : : :"memory" ); - asm volatile ("isync" : : : "memory"); + __asm__volatile ("sync" : : :"memory" ); + __asm__ volatile ("isync" : : : "memory"); }#else
exec-all.h +3 -3
@@ -224,9 +224,9 @@ static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)static inline void tb_set_jmp_target1 (uintptr_t jmp_addr,uintptr_t addr) {#if !QEMU_GNUC_PREREQ (4 ,1 ) - registerunsigned long _beg __asm ("a1" ); - registerunsigned long _end __asm ("a2" ); - registerunsigned long _flg __asm ("a3" ); + registerunsigned long _beg __asm__ ("a1" ); + registerunsigned long _end __asm__ ("a2" ); + registerunsigned long _flg __asm__ ("a3" );#endif /* we could use a ldr pc, [pc, #-4] kind of branch and avoid the flush */
qemu-barrier.h +7 -7
@@ -2,7 +2,7 @@#define __QEMU_BARRIER_H 1/* Compiler barrier */ -#define barrier () asm volatile("" ::: "memory") +#define barrier () __asm__ volatile("" ::: "memory")#if defined(__i386__)
@@ -22,14 +22,14 @@#if defined(__GNUC__) && __GNUC__ >=4 && __GNUC_MINOR__ >=4 #define smp_mb () __sync_synchronize()#else -#define smp_mb () asm volatile("lock; addl $0,0(%%esp) " ::: "memory") +#define smp_mb () __asm__ volatile("lock; addl $0,0(%%esp) " ::: "memory")#endif #elif defined(__x86_64__)#define smp_wmb () barrier()#define smp_rmb () barrier() -#define smp_mb () asm volatile("mfence" ::: "memory") +#define smp_mb () __asm__ volatile("mfence" ::: "memory")#elif defined(_ARCH_PPC)
@@ -38,15 +38,15 @@* need to order cacheable and non-cacheable stores with respect to * each other */ -#define smp_wmb () asm volatile("eieio" ::: "memory") +#define smp_wmb () __asm__ volatile("eieio" ::: "memory")#if defined(__powerpc64__) -#define smp_rmb () asm volatile("lwsync" ::: "memory") +#define smp_rmb () __asm__ volatile("lwsync" ::: "memory")#else -#define smp_rmb () asm volatile("sync" ::: "memory") +#define smp_rmb () __asm__ volatile("sync" ::: "memory")#endif -#define smp_mb () asm volatile("sync" ::: "memory") +#define smp_mb () __asm__ volatile("sync" ::: "memory")#else
qemu-timer.h +9 -9
@@ -174,7 +174,7 @@ static inline int64_t cpu_get_real_ticks(void)static inline int64_t cpu_get_real_ticks (void ) {int64_t val; - asmvolatile ("rdtsc" :"=A" (val)); + __asm__volatile ("rdtsc" :"=A" (val));return val; }
@@ -184,7 +184,7 @@ static inline int64_t cpu_get_real_ticks(void) {uint32_t low,high;int64_t val; - asmvolatile ("rdtsc" :"=a" (low),"=d" (high)); + __asm__volatile ("rdtsc" :"=a" (low),"=d" (high)); val = high; val <<=32 ; val |= low;
@@ -196,7 +196,7 @@ static inline int64_t cpu_get_real_ticks(void)static inline int64_t cpu_get_real_ticks (void ) {int val; - asmvolatile ("mfctl %%cr16, %0" :"=r" (val)); + __asm__volatile ("mfctl %%cr16, %0" :"=r" (val));return val; }
@@ -205,7 +205,7 @@ static inline int64_t cpu_get_real_ticks(void)static inline int64_t cpu_get_real_ticks (void ) {int64_t val; - asmvolatile ("mov %0 = ar.itc" :"=r" (val) ::"memory" ); + __asm__volatile ("mov %0 = ar.itc" :"=r" (val) ::"memory" );return val; }
@@ -214,7 +214,7 @@ static inline int64_t cpu_get_real_ticks(void)static inline int64_t cpu_get_real_ticks (void ) {int64_t val; - asmvolatile ("stck 0(%1)" :"=m" (val) :"a" (&val) :"cc" ); + __asm__volatile ("stck 0(%1)" :"=m" (val) :"a" (&val) :"cc" );return val; }
@@ -224,7 +224,7 @@ static inline int64_t cpu_get_real_ticks (void) {#if defined(_LP64)uint64_t rval; - asmvolatile ("rd %%tick,%0" :"=r" (rval)); + __asm__volatile ("rd %%tick,%0" :"=r" (rval));return rval;#else union {
@@ -234,8 +234,8 @@ static inline int64_t cpu_get_real_ticks (void) uint32_t low; } i32; } rval; - asmvolatile ("rd %%tick,%1; srlx %1,32,%0" - :"=r" (rval.i32.high),"=r" (rval.i32.low)); + __asm__volatile ("rd %%tick,%1; srlx %1,32,%0" + :"=r" (rval.i32.high),"=r" (rval.i32.low));return rval.i64;#endif }
@@ -277,7 +277,7 @@ static inline int64_t cpu_get_real_ticks(void)uint64_t cc;uint32_t cur, ofs; - asmvolatile ("rpcc %0" :"=r" (cc)); + __asm__volatile ("rpcc %0" :"=r" (cc)); cur = cc; ofs = cc >>32 ;return cur - ofs;
target-i386/cpu.c +13 -13
@@ -114,20 +114,20 @@ void host_cpuid(uint32_t function, uint32_t count,uint32_t vec[4 ];#ifdef __x86_64__ - asmvolatile ("cpuid" - :"=a" (vec[0 ]),"=b" (vec[1 ]), -"=c" (vec[2 ]),"=d" (vec[3 ]) - :"0" (function),"c" (count) :"cc" ); + __asm__volatile ("cpuid" + :"=a" (vec[0 ]),"=b" (vec[1 ]), +"=c" (vec[2 ]),"=d" (vec[3 ]) + :"0" (function),"c" (count) :"cc" );#else - asmvolatile ("pusha \n\t" -"cpuid \n\t" -"mov %%eax, 0(%2) \n\t" -"mov %%ebx, 4(%2) \n\t" -"mov %%ecx, 8(%2) \n\t" -"mov %%edx, 12(%2) \n\t" -"popa" - : :"a" (function),"c" (count),"S" (vec) - :"memory" ,"cc" ); + __asm__volatile ("pusha \n\t" +"cpuid \n\t" +"mov %%eax, 0(%2) \n\t" +"mov %%ebx, 4(%2) \n\t" +"mov %%ecx, 8(%2) \n\t" +"mov %%edx, 12(%2) \n\t" +"popa" + : :"a" (function),"c" (count),"S" (vec) + :"memory" ,"cc" );#endif if (eax)
target-ppc/kvm.c +2 -2
@@ -1105,8 +1105,8 @@ static inline uint32_t mfpvr(void) {uint32_t pvr; - asm ("mfpvr %0" - :"=r" (pvr)); + __asm__ ("mfpvr %0" + :"=r" (pvr));return pvr; }
target-ppc/kvm_ppc.h +1 -1
@@ -113,7 +113,7 @@ static inline int kvmppc_fixup_cpu(CPUPPCState *env)#define kvmppc_eieio () \ do { \ if (kvm_enabled()) { \ - asm volatile("eieio" : : : "memory"); \ + __asm__ volatile("eieio" : : : "memory"); \ } \ } while (0)#endif
tcg/arm/tcg-target.h +4 -4
@@ -87,9 +87,9 @@ static inline void flush_icache_range(tcg_target_ulong start,#if QEMU_GNUC_PREREQ (4 ,1 )__builtin___clear_cache ((char *) start, (char *) stop);#else - register unsigned long _beg __asm ("a1") = start; - register unsigned long _end __asm ("a2") = stop; - register unsigned long _flg __asm ("a3") = 0; - __asm __volatile__ ("swi 0x9f0002" : :"r" (_beg),"r" (_end),"r" (_flg)); + register unsigned long _beg __asm__ ("a1") = start; + register unsigned long _end __asm__ ("a2") = stop; + register unsigned long _flg __asm__ ("a3") = 0; + __asm__ __volatile__ ("swi 0x9f0002" : :"r" (_beg),"r" (_end),"r" (_flg));#endif }
tcg/hppa/tcg-target.h +5 -5
@@ -113,11 +113,11 @@ static inline void flush_icache_range(tcg_target_ulong start, { start &= ~31 ;while (start <= stop) { - asmvolatile ("fdc 0(%0)\n\t" -"sync\n\t" -"fic 0(%%sr4, %0)\n\t" -"sync" - : :"r" (start) :"memory" ); + __asm__volatile ("fdc 0(%0)\n\t" +"sync\n\t" +"fic 0(%%sr4, %0)\n\t" +"sync" + : :"r" (start) :"memory" ); start +=32 ; } }
tcg/ia64/tcg-target.h +2 -2
@@ -153,7 +153,7 @@ static inline void flush_icache_range(tcg_target_ulong start, stop = (stop + (32UL -1UL )) & ~(32UL -1UL );for (; start < stop; start +=32UL ) { - asmvolatile ("fc.i %0" ::"r" (start)); + __asm__volatile ("fc.i %0" ::"r" (start)); } - asmvolatile (";;sync.i;;srlz.i;;" ); + __asm__volatile (";;sync.i;;srlz.i;;" ); }
tcg/s390/tcg-target.c +7 -7
@@ -2233,8 +2233,8 @@ static void query_facilities(void)kernel-only, storing its results at absolute address 200. */ /* stfle 0(%r1) */ r1 = &facilities; - asmvolatile (".word 0xb2b0,0x1000" - :"=r" (r0) :"0" (0 ),"r" (r1) :"memory" ,"cc" ); + __asm__volatile (".word 0xb2b0,0x1000" + :"=r" (r0) :"0" (0 ),"r" (r1) :"memory" ,"cc" );if (got_sigill) {/* STORE FACILITY EXTENDED is not available. Probe for one of each
@@ -2246,7 +2246,7 @@ static void query_facilities(void)/* Test for z/Architecture. Required even in 31-bit mode. */ got_sigill =0 ;/* agr %r0,%r0 */ - asmvolatile (".word 0xb908,0x0000" :"=r" (r0) : :"cc" ); + __asm__volatile (".word 0xb908,0x0000" :"=r" (r0) : :"cc" );if (!got_sigill) { facilities |=FACILITY_ZARCH_ACTIVE ; }
@@ -2255,8 +2255,8 @@ static void query_facilities(void) got_sigill =0 ;/* ly %r0,0(%r1) */ r1 = &facilities; - asmvolatile (".word 0xe300,0x1000,0x0058" - :"=r" (r0) :"r" (r1) :"cc" ); + __asm__volatile (".word 0xe300,0x1000,0x0058" + :"=r" (r0) :"r" (r1) :"cc" );if (!got_sigill) { facilities |=FACILITY_LONG_DISP ; }
@@ -2264,7 +2264,7 @@ static void query_facilities(void)/* Test for extended immediates. */ got_sigill =0 ;/* afi %r0,0 */ - asmvolatile (".word 0xc209,0x0000,0x0000" : : :"cc" ); + __asm__volatile (".word 0xc209,0x0000,0x0000" : : :"cc" );if (!got_sigill) { facilities |=FACILITY_EXT_IMM ; }
@@ -2272,7 +2272,7 @@ static void query_facilities(void)/* Test for general-instructions-extension. */ got_sigill =0 ;/* msfi %r0,1 */ - asmvolatile (".word 0xc201,0x0000,0x0001" ); + __asm__volatile (".word 0xc201,0x0000,0x0001" );if (!got_sigill) { facilities |=FACILITY_GEN_INST_EXT ; }
tcg/tcg.c +1 -1
@@ -2294,7 +2294,7 @@ struct jit_descriptor {void __jit_debug_register_code (void ) __attribute__((noinline));void __jit_debug_register_code (void ) { - asm("" ); + __asm__("" ); }/* Must statically initialize the version, because GDB may check
Signature · 1 line
1.7.2.5
PATCH 4/5 Avoid unportable %m format
In reply to [email protected]From: Blue Swirl <[email protected]>
Replace %m format with explicit call to standard strerror().
--- kvm-all.c | 3 ++- main-loop.c | 2 +- net/tap-linux.c | 8 +++++--- qemu-nbd.c | 3 +-- 4 files changed, 9 insertions(+), 7 deletions(-)
kvm-all.c +2 -1
@@ -1219,7 +1219,8 @@ int kvm_init(void) s->vmfd =-1 ; s->fd =qemu_open ("/dev/kvm" ,O_RDWR );if (s->fd ==-1 ) { -fprintf (stderr,"Could not access KVM kernel module: %m\n" ); +fprintf (stderr,"Could not access KVM kernel module: %s\n" , +strerror (errno)); ret = -errno; goto err; }
main-loop.c +1 -1
@@ -116,7 +116,7 @@ static void sigfd_handler(void *opaque) }if (len !=sizeof (info)) { -printf ("read from sigfd returned %zd: %m\n" , len); +printf ("read from sigfd returned %zd: %s\n" , len,strerror (errno));return ; }
net/tap-linux.c +5 -3
@@ -42,7 +42,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_requiredTFR (fd =open (PATH_NET_TUN ,O_RDWR ));if (fd <0 ) { -error_report ("could not open %s: %m" ,PATH_NET_TUN ); +error_report ("could not open %s: %s" ,PATH_NET_TUN ,strerror (errno));return -1 ; }memset (&ifr,0 ,sizeof (ifr));
@@ -74,9 +74,11 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required ret =ioctl (fd,TUNSETIFF , (void *) &ifr);if (ret !=0 ) {if (ifname[0 ] !='\0' ) { -error_report ("could not configure %s (%s): %m" ,PATH_NET_TUN , ifr.ifr_name); +error_report ("could not configure %s (%s): %s" ,PATH_NET_TUN , + ifr.ifr_name,strerror (errno)); }else { -error_report ("could not configure %s: %m" ,PATH_NET_TUN ); +error_report ("could not configure %s: %s" ,PATH_NET_TUN , +strerror (errno)); }close (fd);return -1 ;
qemu-nbd.c +1 -2
@@ -213,8 +213,7 @@ static void *nbd_client_thread(void *arg) fd =open (device,O_RDWR );if (fd <0 ) { -/* Linux-only, we can use %m in printf. */ -fprintf (stderr,"Failed to open %s: %m" , device); +fprintf (stderr,"Failed to open %s: %s" , device,strerror (errno)); goto out; }
Signature · 1 line
1.7.2.5
Re: [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
Quoted 5 lines · On 07/08/2012 06:51 AM, [email protected] wrote:
From: Blue Swirl<[email protected]>
Replace %m format with explicit call to standard
strerror().Signed-off-by Blue Swirl<[email protected]>
I would expect '%m' to be thread safe whereas strerror() isn't. I don't think this change is actually good.
You'd need to do something more clever with strerror_r() to be equivalent.
Regards,
Anthony Liguori
Quoted 75 lines
--- kvm-all.c | 3 ++- main-loop.c | 2 +- net/tap-linux.c | 8 +++++--- qemu-nbd.c | 3 +-- 4 files changed, 9 insertions(+), 7 deletions(-)kvm-all.c +2
-1@@ -1219,7 +1219,8 @@ int kvm_init(void) s->vmfd =-1 ; s->fd =qemu_open ("/dev/kvm" ,O_RDWR );if (s->fd ==-1 ) { -fprintf (stderr,"Could not access KVM kernel module: %m\n" ); +fprintf (stderr,"Could not access KVM kernel module: %s\n" , +strerror (errno)); ret = -errno; goto err; }main-loop.c +1
-1@@ -116,7 +116,7 @@ static void sigfd_handler(void *opaque) }if (len !=sizeof (info)) { -printf ("read from sigfd returned %zd: %m\n" , len); +printf ("read from sigfd returned %zd: %s\n" , len,strerror (errno));return ; }net/tap-linux.c +5
-3@@ -42,7 +42,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_requiredTFR (fd =open (PATH_NET_TUN ,O_RDWR ));if (fd<0 ) { -error_report ("could not open %s: %m" ,PATH_NET_TUN ); +error_report ("could not open %s: %s" ,PATH_NET_TUN ,strerror (errno));return -1 ; }memset (&ifr,0 ,sizeof (ifr));@@ -74,9 +74,11 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required ret =ioctl (fd,TUNSETIFF , (void *)&ifr);if (ret !=0 ) {if (ifname[0 ] !='\0' ) { -error_report ("could not configure %s (%s): %m" ,PATH_NET_TUN , ifr.ifr_name); +error_report ("could not configure %s (%s): %s" ,PATH_NET_TUN , + ifr.ifr_name,strerror (errno)); }else { -error_report ("could not configure %s: %m" ,PATH_NET_TUN ); +error_report ("could not configure %s: %s" ,PATH_NET_TUN , +strerror (errno)); }close (fd);return -1 ;qemu-nbd.c +1
-2@@ -213,8 +213,7 @@ static void *nbd_client_thread(void *arg) fd =open (device,O_RDWR );if (fd<0 ) { -/* Linux-only, we can use %m in printf. */ -fprintf (stderr,"Failed to open %s: %m" , device); +fprintf (stderr,"Failed to open %s: %s" , device,strerror (errno)); goto out; }
Re: [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
Am 10.07.2012 22:53, schrieb Anthony Liguori:
Quoted 7 lines · On 07/08/2012 06:51 AM, [email protected] wrote:
Quoted 5 lines
From: Blue Swirl<[email protected]>
Replace %m format with explicit call to standard
strerror().Signed-off-by Blue Swirl<[email protected]>
Quoted 8 lines
I would expect '%m' to be thread safe whereas
strerror()isn't. I don't think this change is actually good.You'd need to do something more clever with
strerror_r()to be equivalent.Regards,
Anthony Liguori
Extract from the Linux manpage PRINTF(3):
m (Glibc extension.) Print output of strerror(errno). No
argument is required.
The patch should result in identical behaviour with Glibc. It improves the situation for all platforms which don't use Glibc.
In theory, strerror_r() might be more correct, but in the typical practical scenarios (error output before abort) it is not really needed.
Regards,
Stefan Weil
Re: [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
Quoted 13 lines · Stefan Weil
Am 10.07.2012 22:53, schrieb Anthony Liguori:
Quoted 12 lines · On 07/08/2012 06:51 AM, [email protected] wrote:
Quoted 5 lines
From: Blue Swirl<[email protected]>
Replace %m format with explicit call to standard
strerror().Signed-off-by Blue Swirl<[email protected]>I would expect '%m' to be thread safe whereas
strerror()isn't. I don't think this change is actually good.You'd need to do something more clever with
strerror_r()to be equivalent.
Quoted 8 lines
Quoted 4 lines
Regards,
Anthony Liguori
Extract from the Linux manpage PRINTF(3):
m (Glibc extension.) Print output of strerror(errno). No argument is required.
You're reading the docs too literally:
It uses strerror_r() and is thread-safe. Converting to strerror() removes that safety.
Regards,
Anthony Liguori
Re: [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
In reply to Anthony LiguoriQuoted 6 lines · Anthony Liguori
Quoted 5 lines · On 07/08/2012 06:51 AM, [email protected] wrote:
From: Blue Swirl<[email protected]>
Replace %m format with explicit call to standard
strerror().
Quoted 7 lines
Quoted 1 line
Signed-off-by Blue Swirl<[email protected]>I would expect '%m' to be thread safe whereas
strerror()isn't. I don't think this change is actually good.You'd need to do something more clever with
strerror_r()to be equivalent.
There's also g_strerror() which has a simpler interface.
Quoted 88 lines
Regards,
Anthony Liguori
Quoted 81 lines
--- kvm-all.c | 3 ++- main-loop.c | 2 +- net/tap-linux.c | 8 +++++--- qemu-nbd.c | 3 +-- 4 files changed, 9 insertions(+), 7 deletions(-)kvm-all.c +2
-1@@ -1219,7 +1219,8 @@ int kvm_init(void) s->vmfd =-1 ; s->fd =qemu_open ("/dev/kvm" ,O_RDWR );if (s->fd ==-1 ) { -fprintf (stderr,"Could not access KVM kernel module: %m\n" ); +fprintf (stderr,"Could not access KVM kernel module: %s\n" , +strerror (errno)); ret = -errno; goto err; }main-loop.c +1
-1@@ -116,7 +116,7 @@ static void sigfd_handler(void *opaque) }if (len !=sizeof (info)) { -printf ("read from sigfd returned %zd: %m\n" , len); +printf ("read from sigfd returned %zd: %s\n" , len,strerror (errno));return ; }net/tap-linux.c +1
-1@@ -42,7 +42,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_requiredTFR (fd =open (PATH_NET_TUN ,O_RDWR ));if (fd<0 ) { -error_report ("could not open %s: %m" ,PATH_NET_TUN ); +error_report ("could not open %s: %s" ,PATH_NET_TUN ,strerror (errno));return -1 ;} memset(&ifr, 0, sizeof(ifr));net/tap-linux.c +4
-2@@ -74,9 +74,11 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required ret =ioctl (fd,TUNSETIFF , (void *)&ifr);if (ret !=0 ) {if (ifname[0 ] !='\0' ) { -error_report ("could not configure %s (%s): %m" ,PATH_NET_TUN , ifr.ifr_name); +error_report ("could not configure %s (%s): %s" ,PATH_NET_TUN , + ifr.ifr_name,strerror (errno)); }else { -error_report ("could not configure %s: %m" ,PATH_NET_TUN ); +error_report ("could not configure %s: %s" ,PATH_NET_TUN , +strerror (errno)); }close(fd); return -1;qemu-nbd.c +1
-2@@ -213,8 +213,7 @@ static void *nbd_client_thread(void *arg) fd =open (device,O_RDWR );if (fd<0 ) { -/* Linux-only, we can use %m in printf. */ -fprintf (stderr,"Failed to open %s: %m" , device); +fprintf (stderr,"Failed to open %s: %s" , device,strerror (errno)); goto out; }
PATCH 5/5 Avoid redefining inline
In reply to [email protected]From: Blue Swirl <[email protected]>
System headers (e.g. gutils.h) may define 'inline', #undefine it before #defining.
--- osdep.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
osdep.h +2 -0
@@ -70,10 +70,12 @@ typedef signed int int_fast16_t;#ifndef always_inline#if !((__GNUC__ <3 ) || defined(__APPLE__))#ifdef __OPTIMIZE__ +#undef inline#define inline __attribute__ (( always_inline )) __inline__#endif #endif #else +#undef inline#define inline always_inline#endif
Signature · 1 line
1.7.2.5