diff mbox

[1/5] Avoid GCC extension ?:

Message ID 5b88891d9683e0289cd8e24a999ac9d1fdb3fdb3.1341748181.git.blauwirbel@gmail.com
State New
Headers show

Commit Message

Blue Swirl July 8, 2012, 11:51 a.m. UTC
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(-)

Comments

Andreas Schwab July 8, 2012, 12:09 p.m. UTC | #1
blauwirbel@gmail.com writes:

> +    pstrcpy(bs->backing_format, sizeof(bs->backing_format),
> +            backing_fmt ? backing_file : "");

s/backing_file/backing_fmt/

Andreas.
Blue Swirl July 8, 2012, 12:14 p.m. UTC | #2
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."
Markus Armbruster July 9, 2012, 7:35 a.m. UTC | #3
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?
Blue Swirl July 10, 2012, 7:09 p.m. UTC | #4
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.
Kevin Wolf July 11, 2012, 12:54 p.m. UTC | #5
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
Peter Maydell July 11, 2012, 1:09 p.m. UTC | #6
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
陳韋任 July 11, 2012, 1:12 p.m. UTC | #7
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
Blue Swirl July 12, 2012, 8:28 p.m. UTC | #8
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
Blue Swirl July 12, 2012, 8:34 p.m. UTC | #9
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
Peter Maydell July 12, 2012, 9:08 p.m. UTC | #10
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
Kevin Wolf July 13, 2012, 9:05 a.m. UTC | #11
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
Blue Swirl July 13, 2012, 2:47 p.m. UTC | #12
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
Blue Swirl July 13, 2012, 2:58 p.m. UTC | #13
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
diff mbox

Patch

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(&params.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,
                     &params, 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,