We banned use of certain g_assert_FOO() functions outside tests, and
made checkpatch.pl flag them (commit 6e9389563e5). We neglected to
purge existing uses. Do that now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/ahci.c | 2 +-
hw/ppc/spapr_ovec.c | 12 ++++++------
hw/usb/dev-smartcard-reader.c | 2 +-
qom/object.c | 10 +++++-----
util/qht.c | 2 +-
5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 24dbad5125..75cccbea47 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -387,7 +387,7 @@ static uint64_t ahci_mem_read(void *opaque, hwaddr addr, unsigned size)
if (ofst + size <= 4) {
val = lo >> (ofst * 8);
} else {
- g_assert_cmpint(size, >, 1);
+ g_assert(size > 1);
/* If the 64bit read is unaligned, we will produce undefined
* results. AHCI does not support unaligned 64bit reads. */
diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
index 41df4c35ba..318bf33de4 100644
--- a/hw/ppc/spapr_ovec.c
+++ b/hw/ppc/spapr_ovec.c
@@ -113,7 +113,7 @@ void spapr_ovec_cleanup(sPAPROptionVector *ov)
void spapr_ovec_set(sPAPROptionVector *ov, long bitnr)
{
g_assert(ov);
- g_assert_cmpint(bitnr, <, OV_MAXBITS);
+ g_assert(bitnr < OV_MAXBITS);
set_bit(bitnr, ov->bitmap);
}
@@ -121,7 +121,7 @@ void spapr_ovec_set(sPAPROptionVector *ov, long bitnr)
void spapr_ovec_clear(sPAPROptionVector *ov, long bitnr)
{
g_assert(ov);
- g_assert_cmpint(bitnr, <, OV_MAXBITS);
+ g_assert(bitnr < OV_MAXBITS);
clear_bit(bitnr, ov->bitmap);
}
@@ -129,7 +129,7 @@ void spapr_ovec_clear(sPAPROptionVector *ov, long bitnr)
bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr)
{
g_assert(ov);
- g_assert_cmpint(bitnr, <, OV_MAXBITS);
+ g_assert(bitnr < OV_MAXBITS);
return test_bit(bitnr, ov->bitmap) ? true : false;
}
@@ -186,7 +186,7 @@ sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector)
int i;
g_assert(table_addr);
- g_assert_cmpint(vector, >=, 1); /* vector numbering starts at 1 */
+ g_assert(vector >= 1); /* vector numbering starts at 1 */
addr = vector_addr(table_addr, vector);
if (!addr) {
@@ -195,7 +195,7 @@ sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector)
}
vector_len = ldub_phys(&address_space_memory, addr++) + 1;
- g_assert_cmpint(vector_len, <=, OV_MAXBYTES);
+ g_assert(vector_len <= OV_MAXBYTES);
ov = spapr_ovec_new();
for (i = 0; i < vector_len; i++) {
@@ -225,7 +225,7 @@ int spapr_ovec_populate_dt(void *fdt, int fdt_offset,
* encoding/sizing expected in ibm,client-architecture-support
*/
vec_len = (lastbit == OV_MAXBITS) ? 1 : lastbit / BITS_PER_BYTE + 1;
- g_assert_cmpint(vec_len, <=, OV_MAXBYTES);
+ g_assert(vec_len <= OV_MAXBYTES);
/* guest expects vector len encoded as vec_len - 1, since the length byte
* is assumed and not included, and the first byte of the vector
* is assumed as well
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 2131e33d27..13d0befd9c 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -786,7 +786,7 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t slot, uint8_t seq,
DPRINTF(s, D_VERBOSE, "error %d\n", p->b.bError);
}
if (len) {
- g_assert_nonnull(data);
+ assert(data);
memcpy(p->abData, data, len);
}
ccid_reset_error_status(s);
diff --git a/qom/object.c b/qom/object.c
index cb7a8cd589..794bd9a3f5 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -295,7 +295,7 @@ static void type_initialize(TypeImpl *ti)
GSList *e;
int i;
- g_assert_cmpint(parent->class_size, <=, ti->class_size);
+ g_assert(parent->class_size <= ti->class_size);
memcpy(ti->class, parent->class, parent->class_size);
ti->class->interfaces = NULL;
ti->class->properties = g_hash_table_new_full(
@@ -372,9 +372,9 @@ static void object_initialize_with_type(void *data, size_t size, TypeImpl *type)
g_assert(type != NULL);
type_initialize(type);
- g_assert_cmpint(type->instance_size, >=, sizeof(Object));
+ g_assert(type->instance_size >= sizeof(Object));
g_assert(type->abstract == false);
- g_assert_cmpint(size, >=, type->instance_size);
+ g_assert(size >= type->instance_size);
memset(obj, 0, type->instance_size);
obj->class = type->class;
@@ -475,7 +475,7 @@ static void object_finalize(void *data)
object_property_del_all(obj);
object_deinit(obj, ti);
- g_assert_cmpint(obj->ref, ==, 0);
+ g_assert(obj->ref == 0);
if (obj->free) {
obj->free(obj);
}
@@ -917,7 +917,7 @@ void object_unref(Object *obj)
if (!obj) {
return;
}
- g_assert_cmpint(obj->ref, >, 0);
+ g_assert(obj->ref > 0);
/* parent always holds a reference to its children */
if (atomic_fetch_dec(&obj->ref) == 1) {
diff --git a/util/qht.c b/util/qht.c
index ff4d2e6974..55b0738cd1 100644
--- a/util/qht.c
+++ b/util/qht.c
@@ -759,7 +759,7 @@ static void qht_do_resize_reset(struct qht *ht, struct qht_map *new, bool reset)
return;
}
- g_assert_cmpuint(new->n_buckets, !=, old->n_buckets);
+ g_assert(new->n_buckets != old->n_buckets);
qht_map_iter__all_locked(ht, old, qht_map_copy, new);
qht_map_debug__all_locked(new);
--
2.17.1
On Fri, Jun 08, 2018 at 07:02:31PM +0200, Markus Armbruster wrote:
> We banned use of certain g_assert_FOO() functions outside tests, and
> made checkpatch.pl flag them (commit 6e9389563e5). We neglected to
> purge existing uses. Do that now.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/ide/ahci.c | 2 +-
> hw/ppc/spapr_ovec.c | 12 ++++++------
> hw/usb/dev-smartcard-reader.c | 2 +-
> qom/object.c | 10 +++++-----
> util/qht.c | 2 +-
> 5 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index 2131e33d27..13d0befd9c 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -786,7 +786,7 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t slot, uint8_t seq,
> DPRINTF(s, D_VERBOSE, "error %d\n", p->b.bError);
> }
> if (len) {
> - g_assert_nonnull(data);
> + assert(data);
nitpick - all the other conversions used g_assert()
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 06/08/2018 02:12 PM, Daniel P. Berrangé wrote:
> On Fri, Jun 08, 2018 at 07:02:31PM +0200, Markus Armbruster wrote:
>> We banned use of certain g_assert_FOO() functions outside tests, and
>> made checkpatch.pl flag them (commit 6e9389563e5). We neglected to
>> purge existing uses. Do that now.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> hw/ide/ahci.c | 2 +-
>> hw/ppc/spapr_ovec.c | 12 ++++++------
>> hw/usb/dev-smartcard-reader.c | 2 +-
>> qom/object.c | 10 +++++-----
>> util/qht.c | 2 +-
>> 5 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>
>> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
>> index 2131e33d27..13d0befd9c 100644
>> --- a/hw/usb/dev-smartcard-reader.c
>> +++ b/hw/usb/dev-smartcard-reader.c
>> @@ -786,7 +786,7 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t slot, uint8_t seq,
>> DPRINTF(s, D_VERBOSE, "error %d\n", p->b.bError);
>> }
>> if (len) {
>> - g_assert_nonnull(data);
>> + assert(data);
>
> nitpick - all the other conversions used g_assert()
I'd rather use assert() in the whole codebase...
I understand the rational here as "there is no other call to the
glib-Testing API, so keep it simple and directly use assert()"
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>
> Regards,
> Daniel
>
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> On 06/08/2018 02:12 PM, Daniel P. Berrangé wrote:
>> On Fri, Jun 08, 2018 at 07:02:31PM +0200, Markus Armbruster wrote:
>>> We banned use of certain g_assert_FOO() functions outside tests, and
>>> made checkpatch.pl flag them (commit 6e9389563e5). We neglected to
>>> purge existing uses. Do that now.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>> hw/ide/ahci.c | 2 +-
>>> hw/ppc/spapr_ovec.c | 12 ++++++------
>>> hw/usb/dev-smartcard-reader.c | 2 +-
>>> qom/object.c | 10 +++++-----
>>> util/qht.c | 2 +-
>>> 5 files changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>>
>>> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
>>> index 2131e33d27..13d0befd9c 100644
>>> --- a/hw/usb/dev-smartcard-reader.c
>>> +++ b/hw/usb/dev-smartcard-reader.c
>>> @@ -786,7 +786,7 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t slot, uint8_t seq,
>>> DPRINTF(s, D_VERBOSE, "error %d\n", p->b.bError);
>>> }
>>> if (len) {
>>> - g_assert_nonnull(data);
>>> + assert(data);
>>
>> nitpick - all the other conversions used g_assert()
The other conversions are in files that predominantly or exclusively use
g_assert(). This one is in a file that exclusively uses assert().
> I'd rather use assert() in the whole codebase...
Me too.
> I understand the rational here as "there is no other call to the
> glib-Testing API, so keep it simple and directly use assert()"
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Thanks!
On Mon, Jun 11, 2018 at 08:06:49AM +0200, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>
> > On 06/08/2018 02:12 PM, Daniel P. Berrangé wrote:
> >> On Fri, Jun 08, 2018 at 07:02:31PM +0200, Markus Armbruster wrote:
> >>> We banned use of certain g_assert_FOO() functions outside tests, and
> >>> made checkpatch.pl flag them (commit 6e9389563e5). We neglected to
> >>> purge existing uses. Do that now.
> >>>
> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >>> ---
> >>> hw/ide/ahci.c | 2 +-
> >>> hw/ppc/spapr_ovec.c | 12 ++++++------
> >>> hw/usb/dev-smartcard-reader.c | 2 +-
> >>> qom/object.c | 10 +++++-----
> >>> util/qht.c | 2 +-
> >>> 5 files changed, 14 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> >>
> >>> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> >>> index 2131e33d27..13d0befd9c 100644
> >>> --- a/hw/usb/dev-smartcard-reader.c
> >>> +++ b/hw/usb/dev-smartcard-reader.c
> >>> @@ -786,7 +786,7 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t slot, uint8_t seq,
> >>> DPRINTF(s, D_VERBOSE, "error %d\n", p->b.bError);
> >>> }
> >>> if (len) {
> >>> - g_assert_nonnull(data);
> >>> + assert(data);
> >>
> >> nitpick - all the other conversions used g_assert()
>
> The other conversions are in files that predominantly or exclusively use
> g_assert(). This one is in a file that exclusively uses assert().
Ah ok, makes sense then.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 06/08/2018 12:02 PM, Markus Armbruster wrote: > We banned use of certain g_assert_FOO() functions outside tests, and > made checkpatch.pl flag them (commit 6e9389563e5). We neglected to > purge existing uses. Do that now. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 06/08/2018 01:02 PM, Markus Armbruster wrote:
> We banned use of certain g_assert_FOO() functions outside tests, and
> made checkpatch.pl flag them (commit 6e9389563e5). We neglected to
> purge existing uses. Do that now.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
I've apparently got a real knack for using APIs that I ought not to.
The commit message for 6e93... makes sense, though, so this does too.
Reviewed-by: John Snow <jsnow@redhat.com>
> ---
> hw/ide/ahci.c | 2 +-
> hw/ppc/spapr_ovec.c | 12 ++++++------
> hw/usb/dev-smartcard-reader.c | 2 +-
> qom/object.c | 10 +++++-----
> util/qht.c | 2 +-
> 5 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 24dbad5125..75cccbea47 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -387,7 +387,7 @@ static uint64_t ahci_mem_read(void *opaque, hwaddr addr, unsigned size)
> if (ofst + size <= 4) {
> val = lo >> (ofst * 8);
> } else {
> - g_assert_cmpint(size, >, 1);
> + g_assert(size > 1);
>
> /* If the 64bit read is unaligned, we will produce undefined
> * results. AHCI does not support unaligned 64bit reads. */
> diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
> index 41df4c35ba..318bf33de4 100644
> --- a/hw/ppc/spapr_ovec.c
> +++ b/hw/ppc/spapr_ovec.c
> @@ -113,7 +113,7 @@ void spapr_ovec_cleanup(sPAPROptionVector *ov)
> void spapr_ovec_set(sPAPROptionVector *ov, long bitnr)
> {
> g_assert(ov);
> - g_assert_cmpint(bitnr, <, OV_MAXBITS);
> + g_assert(bitnr < OV_MAXBITS);
>
> set_bit(bitnr, ov->bitmap);
> }
> @@ -121,7 +121,7 @@ void spapr_ovec_set(sPAPROptionVector *ov, long bitnr)
> void spapr_ovec_clear(sPAPROptionVector *ov, long bitnr)
> {
> g_assert(ov);
> - g_assert_cmpint(bitnr, <, OV_MAXBITS);
> + g_assert(bitnr < OV_MAXBITS);
>
> clear_bit(bitnr, ov->bitmap);
> }
> @@ -129,7 +129,7 @@ void spapr_ovec_clear(sPAPROptionVector *ov, long bitnr)
> bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr)
> {
> g_assert(ov);
> - g_assert_cmpint(bitnr, <, OV_MAXBITS);
> + g_assert(bitnr < OV_MAXBITS);
>
> return test_bit(bitnr, ov->bitmap) ? true : false;
> }
> @@ -186,7 +186,7 @@ sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector)
> int i;
>
> g_assert(table_addr);
> - g_assert_cmpint(vector, >=, 1); /* vector numbering starts at 1 */
> + g_assert(vector >= 1); /* vector numbering starts at 1 */
>
> addr = vector_addr(table_addr, vector);
> if (!addr) {
> @@ -195,7 +195,7 @@ sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector)
> }
>
> vector_len = ldub_phys(&address_space_memory, addr++) + 1;
> - g_assert_cmpint(vector_len, <=, OV_MAXBYTES);
> + g_assert(vector_len <= OV_MAXBYTES);
> ov = spapr_ovec_new();
>
> for (i = 0; i < vector_len; i++) {
> @@ -225,7 +225,7 @@ int spapr_ovec_populate_dt(void *fdt, int fdt_offset,
> * encoding/sizing expected in ibm,client-architecture-support
> */
> vec_len = (lastbit == OV_MAXBITS) ? 1 : lastbit / BITS_PER_BYTE + 1;
> - g_assert_cmpint(vec_len, <=, OV_MAXBYTES);
> + g_assert(vec_len <= OV_MAXBYTES);
> /* guest expects vector len encoded as vec_len - 1, since the length byte
> * is assumed and not included, and the first byte of the vector
> * is assumed as well
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index 2131e33d27..13d0befd9c 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -786,7 +786,7 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t slot, uint8_t seq,
> DPRINTF(s, D_VERBOSE, "error %d\n", p->b.bError);
> }
> if (len) {
> - g_assert_nonnull(data);
> + assert(data);
> memcpy(p->abData, data, len);
> }
> ccid_reset_error_status(s);
> diff --git a/qom/object.c b/qom/object.c
> index cb7a8cd589..794bd9a3f5 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -295,7 +295,7 @@ static void type_initialize(TypeImpl *ti)
> GSList *e;
> int i;
>
> - g_assert_cmpint(parent->class_size, <=, ti->class_size);
> + g_assert(parent->class_size <= ti->class_size);
> memcpy(ti->class, parent->class, parent->class_size);
> ti->class->interfaces = NULL;
> ti->class->properties = g_hash_table_new_full(
> @@ -372,9 +372,9 @@ static void object_initialize_with_type(void *data, size_t size, TypeImpl *type)
> g_assert(type != NULL);
> type_initialize(type);
>
> - g_assert_cmpint(type->instance_size, >=, sizeof(Object));
> + g_assert(type->instance_size >= sizeof(Object));
> g_assert(type->abstract == false);
> - g_assert_cmpint(size, >=, type->instance_size);
> + g_assert(size >= type->instance_size);
>
> memset(obj, 0, type->instance_size);
> obj->class = type->class;
> @@ -475,7 +475,7 @@ static void object_finalize(void *data)
> object_property_del_all(obj);
> object_deinit(obj, ti);
>
> - g_assert_cmpint(obj->ref, ==, 0);
> + g_assert(obj->ref == 0);
> if (obj->free) {
> obj->free(obj);
> }
> @@ -917,7 +917,7 @@ void object_unref(Object *obj)
> if (!obj) {
> return;
> }
> - g_assert_cmpint(obj->ref, >, 0);
> + g_assert(obj->ref > 0);
>
> /* parent always holds a reference to its children */
> if (atomic_fetch_dec(&obj->ref) == 1) {
> diff --git a/util/qht.c b/util/qht.c
> index ff4d2e6974..55b0738cd1 100644
> --- a/util/qht.c
> +++ b/util/qht.c
> @@ -759,7 +759,7 @@ static void qht_do_resize_reset(struct qht *ht, struct qht_map *new, bool reset)
> return;
> }
>
> - g_assert_cmpuint(new->n_buckets, !=, old->n_buckets);
> + g_assert(new->n_buckets != old->n_buckets);
> qht_map_iter__all_locked(ht, old, qht_map_copy, new);
> qht_map_debug__all_locked(new);
>
>
--
—js
© 2016 - 2024 Red Hat, Inc.