diff mbox

[4/7] a trivial code change for more idiomatic writing style

Message ID 1406809740-10836-5-git-send-email-arei.gonglei@huawei.com
State New
Headers show

Commit Message

Gonglei (Arei) July 31, 2014, 12:28 p.m. UTC 
From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 qdev-monitor.c      | 2 +-
 qemu-char.c         | 2 +-
 util/qemu-sockets.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Dr. David Alan Gilbert July 31, 2014, 1:55 p.m. UTC | #1 
* arei.gonglei@huawei.com (arei.gonglei@huawei.com) wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  qdev-monitor.c      | 2 +-
>  qemu-char.c         | 2 +-
>  util/qemu-sockets.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index f87f3d8..3e30d38 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -694,7 +694,7 @@ void qmp_device_del(const char *id, Error **errp)
>      DeviceState *dev;
>  
>      dev = qdev_find_recursive(sysbus_get_default(), id);
> -    if (NULL == dev) {
> +    if (dev == NULL) {

I know people who write it as 'NULL == dev' on purpose,
because that will cause an error if you accidentally type a single =
where as 'dev = NULL'  will just cause confusion.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Peter Maydell July 31, 2014, 1:59 p.m. UTC | #2 
On 31 July 2014 14:55, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * arei.gonglei@huawei.com (arei.gonglei@huawei.com) wrote:
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -694,7 +694,7 @@ void qmp_device_del(const char *id, Error **errp)
>>      DeviceState *dev;
>>
>>      dev = qdev_find_recursive(sysbus_get_default(), id);
>> -    if (NULL == dev) {
>> +    if (dev == NULL) {
>
> I know people who write it as 'NULL == dev' on purpose,
> because that will cause an error if you accidentally type a single =
> where as 'dev = NULL'  will just cause confusion.

Yes, this is the motivation for Yoda conditionals. But it only
makes sense if you don't have a compiler with a sensible
warning configuration. For QEMU you will get an error if you
write "dev = NULL" :

error: suggest parentheses around assignment used as truth value
[-Werror=parentheses]

so we don't need to get people to contort their code like this.

thanks
-- PMM
Dr. David Alan Gilbert July 31, 2014, 2:49 p.m. UTC | #3 
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 31 July 2014 14:55, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * arei.gonglei@huawei.com (arei.gonglei@huawei.com) wrote:
> >> --- a/qdev-monitor.c
> >> +++ b/qdev-monitor.c
> >> @@ -694,7 +694,7 @@ void qmp_device_del(const char *id, Error **errp)
> >>      DeviceState *dev;
> >>
> >>      dev = qdev_find_recursive(sysbus_get_default(), id);
> >> -    if (NULL == dev) {
> >> +    if (dev == NULL) {
> >
> > I know people who write it as 'NULL == dev' on purpose,
> > because that will cause an error if you accidentally type a single =
> > where as 'dev = NULL'  will just cause confusion.
> 
> Yes, this is the motivation for Yoda conditionals. But it only
> makes sense if you don't have a compiler with a sensible
> warning configuration. For QEMU you will get an error if you
> write "dev = NULL" :
> 
> error: suggest parentheses around assignment used as truth value
> [-Werror=parentheses]
> 
> so we don't need to get people to contort their code like this.


OK, that's fair enough.

Dave

> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox

Patch

diff --git a/qdev-monitor.c b/qdev-monitor.c
index f87f3d8..3e30d38 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -694,7 +694,7 @@  void qmp_device_del(const char *id, Error **errp)
     DeviceState *dev;
 
     dev = qdev_find_recursive(sysbus_get_default(), id);
-    if (NULL == dev) {
+    if (dev == NULL) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, id);
         return;
     }
diff --git a/qemu-char.c b/qemu-char.c
index 956be49..70d5a64 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4117,7 +4117,7 @@  void qmp_chardev_remove(const char *id, Error **errp)
     CharDriverState *chr;
 
     chr = qemu_chr_find(id);
-    if (NULL == chr) {
+    if (chr == NULL) {
         error_setg(errp, "Chardev '%s' not found", id);
         return;
     }
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 74cf078..5d38395 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -732,7 +732,7 @@  int unix_connect_opts(QemuOpts *opts, Error **errp,
     ConnectState *connect_state = NULL;
     int sock, rc;
 
-    if (NULL == path) {
+    if (path == NULL) {
         error_setg(errp, "unix connect: no path specified");
         return -1;
     }