* [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-08 11:51 [Qemu-devel] [PATCH 0/5] portability patches blauwirbel
@ 2012-07-08 11:51 ` blauwirbel
2012-07-08 12:09 ` Andreas Schwab
2012-07-09 7:35 ` Markus Armbruster
2012-07-08 11:51 ` [Qemu-devel] [PATCH 2/5] Avoid returning void blauwirbel
` (3 subsequent siblings)
4 siblings, 2 replies; 23+ messages in thread
From: blauwirbel @ 2012-07-08 11:51 UTC (permalink / raw)
To: qemu-devel; +Cc: blueswirl
From: Blue Swirl <blauwirbel@gmail.com>
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 <blauwirbel@gmail.com>
---
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(-)
diff --git a/block.c b/block.c
index 0acdcac..1474633 100644
--- a/block.c
+++ b/block.c
@@ -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;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 2c1cd0a..be7834c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -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);
}
diff --git a/bt-vhci.c b/bt-vhci.c
index bbc1029..ca0bfa8 100644
--- a/bt-vhci.c
+++ b/bt-vhci.c
@@ -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;
diff --git a/hw/bt-hci.c b/hw/bt-hci.c
index a3a7fb4..f436d9e 100644
--- a/hw/bt-hci.c
+++ b/hw/bt-hci.c
@@ -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));
diff --git a/hw/bt.c b/hw/bt.c
index dc99fc2..5e445f5 100644
--- a/hw/bt.c
+++ b/hw/bt.c
@@ -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);
}
diff --git a/hw/gumstix.c b/hw/gumstix.c
index 13a36ea..5112fc2 100644
--- a/hw/gumstix.c
+++ b/hw/gumstix.c
@@ -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) {
diff --git a/hw/omap2.c b/hw/omap2.c
index 4278dd1..9518d0b 100644
--- a/hw/omap2.c
+++ b/hw/omap2.c
@@ -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);
diff --git a/hw/omap_clk.c b/hw/omap_clk.c
index 8448006..0234664 100644
--- a/hw/omap_clk.c
+++ b/hw/omap_clk.c
@@ -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 ++) {
diff --git a/hw/omap_uart.c b/hw/omap_uart.c
index 167d5c4..f84a88c 100644
--- a/hw/omap_uart.c
+++ b/hw/omap_uart.c
@@ -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);
}
diff --git a/hw/onenand.c b/hw/onenand.c
index db6af68..6b933d8 100644
--- a/hw/onenand.c
+++ b/hw/onenand.c
@@ -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 */
diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index b711b6b..f9ea5f5 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -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);
}
}
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 7915b45..c7a6b80 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -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));
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0b89462..6566127 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -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;
}
diff --git a/hw/qdev.c b/hw/qdev.c
index af54467..d183f57 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -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,
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index ae25194..4a51314 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -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);
diff --git a/hw/tsc210x.c b/hw/tsc210x.c
index 3c448a6..c2b8db6 100644
--- a/hw/tsc210x.c
+++ b/hw/tsc210x.c
@@ -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)
diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 8fd30df..e71c0b9 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -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,
diff --git a/hw/wm8750.c b/hw/wm8750.c
index 11bcec3..fd9d776 100644
--- a/hw/wm8750.c
+++ b/hw/wm8750.c
@@ -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;
}
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index e6bb2f2..7976800 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -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;
}
diff --git a/path.c b/path.c
index ef3f277..aa0c563 100644
--- a/path.c
+++ b/path.c
@@ -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;
}
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 071b6be..3a8c2f1 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -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);
diff --git a/vl.c b/vl.c
index 1329c30..6a341fb 100644
--- a/vl.c
+++ b/vl.c
@@ -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,
--
1.7.2.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-08 11:51 ` [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?: blauwirbel
@ 2012-07-08 12:09 ` Andreas Schwab
2012-07-08 12:14 ` Blue Swirl
2012-07-11 12:54 ` Kevin Wolf
2012-07-09 7:35 ` Markus Armbruster
1 sibling, 2 replies; 23+ messages in thread
From: Andreas Schwab @ 2012-07-08 12:09 UTC (permalink / raw)
To: blauwirbel; +Cc: blueswirl, qemu-devel
blauwirbel@gmail.com writes:
> + pstrcpy(bs->backing_format, sizeof(bs->backing_format),
> + backing_fmt ? backing_file : "");
s/backing_file/backing_fmt/
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-08 12:09 ` Andreas Schwab
@ 2012-07-08 12:14 ` Blue Swirl
2012-07-11 12:54 ` Kevin Wolf
1 sibling, 0 replies; 23+ messages in thread
From: Blue Swirl @ 2012-07-08 12:14 UTC (permalink / raw)
To: Andreas Schwab; +Cc: qemu-devel
On Sun, Jul 8, 2012 at 12:09 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> blauwirbel@gmail.com writes:
>
>> + 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.
>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-08 12:09 ` Andreas Schwab
2012-07-08 12:14 ` Blue Swirl
@ 2012-07-11 12:54 ` Kevin Wolf
2012-07-11 13:09 ` Peter Maydell
2012-07-12 20:28 ` Blue Swirl
1 sibling, 2 replies; 23+ messages in thread
From: Kevin Wolf @ 2012-07-11 12:54 UTC (permalink / raw)
To: Andreas Schwab; +Cc: blauwirbel, blueswirl, qemu-devel
Am 08.07.2012 14:09, schrieb Andreas Schwab:
> blauwirbel@gmail.com writes:
>
>> + 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-11 12:54 ` Kevin Wolf
@ 2012-07-11 13:09 ` Peter Maydell
2012-07-11 13:12 ` 陳韋任 (Wei-Ren Chen)
2012-07-12 20:28 ` Blue Swirl
1 sibling, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2012-07-11 13:09 UTC (permalink / raw)
To: Kevin Wolf; +Cc: blauwirbel, blueswirl, Andreas Schwab, qemu-devel
On 11 July 2012 13:54, Kevin Wolf <kwolf@redhat.com> wrote:
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-11 13:09 ` Peter Maydell
@ 2012-07-11 13:12 ` 陳韋任 (Wei-Ren Chen)
2012-07-12 20:34 ` Blue Swirl
0 siblings, 1 reply; 23+ messages in thread
From: 陳韋任 (Wei-Ren Chen) @ 2012-07-11 13:12 UTC (permalink / raw)
To: Peter Maydell
Cc: Kevin Wolf, blauwirbel, Andreas Schwab, qemu-devel, blueswirl
On Wed, Jul 11, 2012 at 02:09:53PM +0100, Peter Maydell wrote:
> On 11 July 2012 13:54, Kevin Wolf <kwolf@redhat.com> wrote:
> > 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
--
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-11 13:12 ` 陳韋任 (Wei-Ren Chen)
@ 2012-07-12 20:34 ` Blue Swirl
0 siblings, 0 replies; 23+ messages in thread
From: Blue Swirl @ 2012-07-12 20:34 UTC (permalink / raw)
To: 陳韋任 (Wei-Ren Chen)
Cc: Kevin Wolf, Peter Maydell, Andreas Schwab, qemu-devel, blueswirl
On Wed, Jul 11, 2012 at 1:12 PM, 陳韋任 (Wei-Ren Chen)
<chenwj@iis.sinica.edu.tw> wrote:
> On Wed, Jul 11, 2012 at 02:09:53PM +0100, Peter Maydell wrote:
>> On 11 July 2012 13:54, Kevin Wolf <kwolf@redhat.com> wrote:
>> > 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. ;)
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.
>
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-11 12:54 ` Kevin Wolf
2012-07-11 13:09 ` Peter Maydell
@ 2012-07-12 20:28 ` Blue Swirl
2012-07-12 21:08 ` Peter Maydell
2012-07-13 9:05 ` Kevin Wolf
1 sibling, 2 replies; 23+ messages in thread
From: Blue Swirl @ 2012-07-12 20:28 UTC (permalink / raw)
To: Kevin Wolf; +Cc: blueswirl, Andreas Schwab, qemu-devel
On Wed, Jul 11, 2012 at 12:54 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 08.07.2012 14:09, schrieb Andreas Schwab:
>> blauwirbel@gmail.com writes:
>>
>>> + 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.
>
> 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.
>
> Kevin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-12 20:28 ` Blue Swirl
@ 2012-07-12 21:08 ` Peter Maydell
2012-07-13 14:47 ` Blue Swirl
2012-07-13 9:05 ` Kevin Wolf
1 sibling, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2012-07-12 21:08 UTC (permalink / raw)
To: Blue Swirl; +Cc: Kevin Wolf, blueswirl, Andreas Schwab, qemu-devel
On 12 July 2012 21:28, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Wed, Jul 11, 2012 at 12:54 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-12 21:08 ` Peter Maydell
@ 2012-07-13 14:47 ` Blue Swirl
0 siblings, 0 replies; 23+ messages in thread
From: Blue Swirl @ 2012-07-13 14:47 UTC (permalink / raw)
To: Peter Maydell; +Cc: Kevin Wolf, blueswirl, Andreas Schwab, qemu-devel
On Thu, Jul 12, 2012 at 9:08 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 12 July 2012 21:28, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Wed, Jul 11, 2012 at 12:54 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> 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.
Fine: there is the demonstrated clear benefit of improved standards compliance.
>
> -- PMM
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-12 20:28 ` Blue Swirl
2012-07-12 21:08 ` Peter Maydell
@ 2012-07-13 9:05 ` Kevin Wolf
2012-07-13 14:58 ` Blue Swirl
1 sibling, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2012-07-13 9:05 UTC (permalink / raw)
To: Blue Swirl; +Cc: blueswirl, Andreas Schwab, qemu-devel
Am 12.07.2012 22:28, schrieb Blue Swirl:
> On Wed, Jul 11, 2012 at 12:54 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 08.07.2012 14:09, schrieb Andreas Schwab:
>>> blauwirbel@gmail.com writes:
>>>
>>>> + 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.
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.
>> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-13 9:05 ` Kevin Wolf
@ 2012-07-13 14:58 ` Blue Swirl
0 siblings, 0 replies; 23+ messages in thread
From: Blue Swirl @ 2012-07-13 14:58 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Andreas Schwab, qemu-devel
On Fri, Jul 13, 2012 at 9:05 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 12.07.2012 22:28, schrieb Blue Swirl:
>> On Wed, Jul 11, 2012 at 12:54 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> Am 08.07.2012 14:09, schrieb Andreas Schwab:
>>>> blauwirbel@gmail.com writes:
>>>>
>>>>> + 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.
>
> 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.
>
>>> 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.
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.
>
> Kevin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-08 11:51 ` [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?: blauwirbel
2012-07-08 12:09 ` Andreas Schwab
@ 2012-07-09 7:35 ` Markus Armbruster
2012-07-10 19:09 ` Blue Swirl
1 sibling, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2012-07-09 7:35 UTC (permalink / raw)
To: blauwirbel; +Cc: blueswirl, qemu-devel
blauwirbel@gmail.com writes:
> From: Blue Swirl <blauwirbel@gmail.com>
>
> 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 <blauwirbel@gmail.com>
> ---
> 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(-)
Practical benefits?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
2012-07-09 7:35 ` Markus Armbruster
@ 2012-07-10 19:09 ` Blue Swirl
0 siblings, 0 replies; 23+ messages in thread
From: Blue Swirl @ 2012-07-10 19:09 UTC (permalink / raw)
To: Markus Armbruster; +Cc: blueswirl, qemu-devel
On Mon, Jul 9, 2012 at 7:35 AM, Markus Armbruster <armbru@redhat.com> wrote:
> blauwirbel@gmail.com writes:
>
>> From: Blue Swirl <blauwirbel@gmail.com>
>>
>> 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 <blauwirbel@gmail.com>
>> ---
>> 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(-)
>
> Practical benefits?
Improved C99 compliance. ?: isn't in C11 either, unlike for example
anonymous unions.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 2/5] Avoid returning void
2012-07-08 11:51 [Qemu-devel] [PATCH 0/5] portability patches blauwirbel
2012-07-08 11:51 ` [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?: blauwirbel
@ 2012-07-08 11:51 ` blauwirbel
2012-07-08 11:51 ` [Qemu-devel] [PATCH 3/5] Use __asm__ instead of asm or __asm blauwirbel
` (2 subsequent siblings)
4 siblings, 0 replies; 23+ messages in thread
From: blauwirbel @ 2012-07-08 11:51 UTC (permalink / raw)
To: qemu-devel; +Cc: blueswirl
From: Blue Swirl <blauwirbel@gmail.com>
It's silly and non-conforming to standards to return void,
don't do it.
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
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(-)
diff --git a/block.c b/block.c
index 1474633..14e91a1 100644
--- a/block.c
+++ b/block.c
@@ -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);
}
diff --git a/dma.h b/dma.h
index acacf1b..f35c4b6 100644
--- a/dma.h
+++ b/dma.h
@@ -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);
}
diff --git a/hw/bt-l2cap.c b/hw/bt-l2cap.c
index 2ccba60..cb43ee7 100644
--- a/hw/bt-l2cap.c
+++ b/hw/bt-l2cap.c
@@ -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
diff --git a/hw/eepro100.c b/hw/eepro100.c
index 6279ae3..0f1f0f4 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -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();
}
}
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index bf8ece4..087b4f9 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -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);
}
}
}
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index f5a74c2..6652761 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -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;
diff --git a/hw/ide/via.c b/hw/ide/via.c
index eec5136..a17f2e2 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -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;
diff --git a/hw/lan9118.c b/hw/lan9118.c
index 7b4fe87..0c354e9 100644
--- a/hw/lan9118.c
+++ b/hw/lan9118.c
@@ -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);
diff --git a/hw/ne2000.c b/hw/ne2000.c
index d02e60c..ccf7715 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -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);
}
}
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 476dc89..f5e4f44 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -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;
}
}
diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index 674ba97..c59b188 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -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 */
diff --git a/ui/vnc.c b/ui/vnc.c
index cf1cae2..cfc61a7 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -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);
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PATCH 3/5] Use __asm__ instead of asm or __asm
2012-07-08 11:51 [Qemu-devel] [PATCH 0/5] portability patches blauwirbel
2012-07-08 11:51 ` [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?: blauwirbel
2012-07-08 11:51 ` [Qemu-devel] [PATCH 2/5] Avoid returning void blauwirbel
@ 2012-07-08 11:51 ` blauwirbel
2012-07-08 11:51 ` [Qemu-devel] [PATCH 4/5] Avoid unportable %m format blauwirbel
2012-07-08 11:51 ` [Qemu-devel] [PATCH 5/5] Avoid redefining inline blauwirbel
4 siblings, 0 replies; 23+ messages in thread
From: blauwirbel @ 2012-07-08 11:51 UTC (permalink / raw)
To: qemu-devel; +Cc: blueswirl
From: Blue Swirl <blauwirbel@gmail.com>
Replace asm and __asm with __asm__.
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
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(-)
diff --git a/bswap.h b/bswap.h
index cc7f84d..7b33d9e 100644
--- a/bswap.h
+++ b/bswap.h
@@ -526,10 +526,10 @@ static inline int lduw_be_p(const void *ptr)
{
#if defined(__i386__)
int val;
- asm volatile ("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;
- asm volatile ("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;
- asm volatile ("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__)
- asm volatile ("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__)
- asm volatile ("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;
diff --git a/cache-utils.h b/cache-utils.h
index 2c57f78..a32cbd2 100644
--- a/cache-utils.h
+++ b/cache-utils.h
@@ -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) {
- asm volatile ("dcbst 0,%0" : : "r"(p) : "memory");
+ __asm__ volatile ("dcbst 0,%0" : : "r"(p) : "memory");
}
- asm volatile ("sync" : : : "memory");
+ __asm__ volatile ("sync" : : : "memory");
start &= start & ~(isize - 1);
stop1 = (stop + isize - 1) & ~(isize - 1);
for (p = start1; p < stop1; p += isize) {
- asm volatile ("icbi 0,%0" : : "r"(p) : "memory");
+ __asm__ volatile ("icbi 0,%0" : : "r"(p) : "memory");
}
- asm volatile ("sync" : : : "memory");
- asm volatile ("isync" : : : "memory");
+ __asm__ volatile ("sync" : : : "memory");
+ __asm__ volatile ("isync" : : : "memory");
}
#else
diff --git a/exec-all.h b/exec-all.h
index 9bda7f7..994d8b9 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -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)
- register unsigned long _beg __asm ("a1");
- register unsigned long _end __asm ("a2");
- register unsigned long _flg __asm ("a3");
+ register unsigned long _beg __asm__ ("a1");
+ register unsigned long _end __asm__ ("a2");
+ register unsigned long _flg __asm__ ("a3");
#endif
/* we could use a ldr pc, [pc, #-4] kind of branch and avoid the flush */
diff --git a/qemu-barrier.h b/qemu-barrier.h
index 7e11197..989a91a 100644
--- a/qemu-barrier.h
+++ b/qemu-barrier.h
@@ -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
diff --git a/qemu-timer.h b/qemu-timer.h
index f8af595..0aca342 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -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;
- asm volatile ("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;
- asm volatile("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;
- asm volatile ("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;
- asm volatile ("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;
- asm volatile("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;
- asm volatile("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;
- asm volatile("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;
- asm volatile("rpcc %0" : "=r"(cc));
+ __asm__ volatile("rpcc %0" : "=r"(cc));
cur = cc;
ofs = cc >> 32;
return cur - ofs;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5521709..2d4acaa 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -114,20 +114,20 @@ void host_cpuid(uint32_t function, uint32_t count,
uint32_t vec[4];
#ifdef __x86_64__
- asm volatile("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
- 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");
+ __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)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 829e180..249a18d 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -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;
}
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index e2f8703..e83cece 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -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
diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index f90b834..15dd35e 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -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
}
diff --git a/tcg/hppa/tcg-target.h b/tcg/hppa/tcg-target.h
index d4bf6fe..402bebf 100644
--- a/tcg/hppa/tcg-target.h
+++ b/tcg/hppa/tcg-target.h
@@ -113,11 +113,11 @@ static inline void flush_icache_range(tcg_target_ulong start,
{
start &= ~31;
while (start <= stop) {
- asm volatile ("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;
}
}
diff --git a/tcg/ia64/tcg-target.h b/tcg/ia64/tcg-target.h
index 0631b9f..7b642e1 100644
--- a/tcg/ia64/tcg-target.h
+++ b/tcg/ia64/tcg-target.h
@@ -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) {
- asm volatile ("fc.i %0" :: "r" (start));
+ __asm__ volatile ("fc.i %0" :: "r" (start));
}
- asm volatile (";;sync.i;;srlz.i;;");
+ __asm__ volatile (";;sync.i;;srlz.i;;");
}
diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
index 04662c1..ae368df 100644
--- a/tcg/s390/tcg-target.c
+++ b/tcg/s390/tcg-target.c
@@ -2233,8 +2233,8 @@ static void query_facilities(void)
kernel-only, storing its results at absolute address 200. */
/* stfle 0(%r1) */
r1 = &facilities;
- asm volatile(".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 */
- asm volatile(".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;
- asm volatile(".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 */
- asm volatile(".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 */
- asm volatile(".word 0xc201,0x0000,0x0001");
+ __asm__ volatile(".word 0xc201,0x0000,0x0001");
if (!got_sigill) {
facilities |= FACILITY_GEN_INST_EXT;
}
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 8386b70..3dcc4d7 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -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
--
1.7.2.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
2012-07-08 11:51 [Qemu-devel] [PATCH 0/5] portability patches blauwirbel
` (2 preceding siblings ...)
2012-07-08 11:51 ` [Qemu-devel] [PATCH 3/5] Use __asm__ instead of asm or __asm blauwirbel
@ 2012-07-08 11:51 ` blauwirbel
2012-07-10 20:53 ` Anthony Liguori
2012-07-08 11:51 ` [Qemu-devel] [PATCH 5/5] Avoid redefining inline blauwirbel
4 siblings, 1 reply; 23+ messages in thread
From: blauwirbel @ 2012-07-08 11:51 UTC (permalink / raw)
To: qemu-devel; +Cc: blueswirl
From: Blue Swirl <blauwirbel@gmail.com>
Replace %m format with explicit call to standard strerror().
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
kvm-all.c | 3 ++-
main-loop.c | 2 +-
net/tap-linux.c | 8 +++++---
qemu-nbd.c | 3 +--
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index f8e4328..d6f4819 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -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;
}
diff --git a/main-loop.c b/main-loop.c
index eb3b6e6..472c55e 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -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;
}
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 41d581b..94e5b1e 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -42,7 +42,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
TFR(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;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 5a0300e..099b5ed 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -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;
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
2012-07-08 11:51 ` [Qemu-devel] [PATCH 4/5] Avoid unportable %m format blauwirbel
@ 2012-07-10 20:53 ` Anthony Liguori
2012-07-10 21:12 ` Stefan Weil
2012-07-12 19:43 ` Blue Swirl
0 siblings, 2 replies; 23+ messages in thread
From: Anthony Liguori @ 2012-07-10 20:53 UTC (permalink / raw)
To: blauwirbel; +Cc: blueswirl, qemu-devel
On 07/08/2012 06:51 AM, blauwirbel@gmail.com wrote:
> From: Blue Swirl<blauwirbel@gmail.com>
>
> Replace %m format with explicit call to standard strerror().
>
> Signed-off-by: Blue Swirl<blauwirbel@gmail.com>
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
> ---
> kvm-all.c | 3 ++-
> main-loop.c | 2 +-
> net/tap-linux.c | 8 +++++---
> qemu-nbd.c | 3 +--
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index f8e4328..d6f4819 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -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;
> }
> diff --git a/main-loop.c b/main-loop.c
> index eb3b6e6..472c55e 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -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;
> }
>
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index 41d581b..94e5b1e 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -42,7 +42,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
>
> TFR(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;
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 5a0300e..099b5ed 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -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;
> }
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
2012-07-10 20:53 ` Anthony Liguori
@ 2012-07-10 21:12 ` Stefan Weil
2012-07-10 21:23 ` Anthony Liguori
2012-07-12 19:43 ` Blue Swirl
1 sibling, 1 reply; 23+ messages in thread
From: Stefan Weil @ 2012-07-10 21:12 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Blue Swirl, qemu-devel
Am 10.07.2012 22:53, schrieb Anthony Liguori:
> On 07/08/2012 06:51 AM, blauwirbel@gmail.com wrote:
>> From: Blue Swirl<blauwirbel@gmail.com>
>>
>> Replace %m format with explicit call to standard strerror().
>>
>> Signed-off-by: Blue Swirl<blauwirbel@gmail.com>
>
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
2012-07-10 21:12 ` Stefan Weil
@ 2012-07-10 21:23 ` Anthony Liguori
0 siblings, 0 replies; 23+ messages in thread
From: Anthony Liguori @ 2012-07-10 21:23 UTC (permalink / raw)
To: Stefan Weil; +Cc: Blue Swirl, qemu-devel
On 07/10/2012 04:12 PM, Stefan Weil wrote:
> Am 10.07.2012 22:53, schrieb Anthony Liguori:
>> On 07/08/2012 06:51 AM, blauwirbel@gmail.com wrote:
>>> From: Blue Swirl<blauwirbel@gmail.com>
>>>
>>> Replace %m format with explicit call to standard strerror().
>>>
>>> Signed-off-by: Blue Swirl<blauwirbel@gmail.com>
>>
>> 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.
You're reading the docs too literally:
http://sourceware.org/git/?p=glibc.git;a=blob;f=stdio-common/vfprintf.c;h=d5690342536bc8cf948c786f663bb63f73f91f3a;hb=HEAD#l966
It uses strerror_r() and is thread-safe. Converting to strerror() removes that
safety.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] Avoid unportable %m format
2012-07-10 20:53 ` Anthony Liguori
2012-07-10 21:12 ` Stefan Weil
@ 2012-07-12 19:43 ` Blue Swirl
1 sibling, 0 replies; 23+ messages in thread
From: Blue Swirl @ 2012-07-12 19:43 UTC (permalink / raw)
To: Anthony Liguori; +Cc: blueswirl, qemu-devel
On Tue, Jul 10, 2012 at 8:53 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 07/08/2012 06:51 AM, blauwirbel@gmail.com wrote:
>>
>> From: Blue Swirl<blauwirbel@gmail.com>
>>
>> Replace %m format with explicit call to standard strerror().
>>
>> Signed-off-by: Blue Swirl<blauwirbel@gmail.com>
>
>
> 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.
>
> Regards,
>
> Anthony Liguori
>
>
>> ---
>> kvm-all.c | 3 ++-
>> main-loop.c | 2 +-
>> net/tap-linux.c | 8 +++++---
>> qemu-nbd.c | 3 +--
>> 4 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index f8e4328..d6f4819 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -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;
>> }
>> diff --git a/main-loop.c b/main-loop.c
>> index eb3b6e6..472c55e 100644
>> --- a/main-loop.c
>> +++ b/main-loop.c
>> @@ -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;
>> }
>>
>> diff --git a/net/tap-linux.c b/net/tap-linux.c
>> index 41d581b..94e5b1e 100644
>> --- a/net/tap-linux.c
>> +++ b/net/tap-linux.c
>> @@ -42,7 +42,7 @@ int tap_open(char *ifname, int ifname_size, int
>> *vnet_hdr, int vnet_hdr_required
>>
>> TFR(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;
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index 5a0300e..099b5ed 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -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;
>> }
>>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 5/5] Avoid redefining inline
2012-07-08 11:51 [Qemu-devel] [PATCH 0/5] portability patches blauwirbel
` (3 preceding siblings ...)
2012-07-08 11:51 ` [Qemu-devel] [PATCH 4/5] Avoid unportable %m format blauwirbel
@ 2012-07-08 11:51 ` blauwirbel
4 siblings, 0 replies; 23+ messages in thread
From: blauwirbel @ 2012-07-08 11:51 UTC (permalink / raw)
To: qemu-devel; +Cc: blueswirl
From: Blue Swirl <blauwirbel@gmail.com>
System headers (e.g. gutils.h) may define 'inline', #undefine it
before #defining.
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
osdep.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/osdep.h b/osdep.h
index 3ea4af0..1e15a4b 100644
--- a/osdep.h
+++ b/osdep.h
@@ -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
--
1.7.2.5
^ permalink raw reply related [flat|nested] 23+ messages in thread