| Message ID | 20201116224143.1284278-5-ehabkost@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qom: Use qlit to represent property defaults | expand |
On Tue, Nov 17, 2020 at 2:42 AM Eduardo Habkost <ehabkost@redhat.com> wrote: > Extract the QNum value comparison logic to a function that takes > QNumValue* as argument. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > include/qapi/qmp/qnum.h | 1 + > qobject/qnum.c | 29 +++++++++++++++++++---------- > 2 files changed, 20 insertions(+), 10 deletions(-) > > diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h > index 62fbdfda68..0327ecd0f0 100644 > --- a/include/qapi/qmp/qnum.h > +++ b/include/qapi/qmp/qnum.h > @@ -106,6 +106,7 @@ double qnum_get_double(const QNum *qn); > > char *qnum_to_string(QNum *qn); > > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue *num_y); > bool qnum_is_equal(const QObject *x, const QObject *y); > void qnum_destroy_obj(QObject *obj); > > diff --git a/qobject/qnum.c b/qobject/qnum.c > index f80d4efd76..6a0f948b16 100644 > --- a/qobject/qnum.c > +++ b/qobject/qnum.c > @@ -207,9 +207,9 @@ char *qnum_to_string(QNum *qn) > } > > /** > - * qnum_is_equal(): Test whether the two QNums are equal > - * @x: QNum object > - * @y: QNum object > + * qnum_value_is_equal(): Test whether two QNumValues are equal > + * @num_x: QNum value > + * @num_y: QNum value > val_x: a QNumValue ? * > * Negative integers are never considered equal to unsigned integers, > * but positive integers in the range [0, INT64_MAX] are considered > @@ -217,13 +217,8 @@ char *qnum_to_string(QNum *qn) > * > * Doubles are never considered equal to integers. > */ > -bool qnum_is_equal(const QObject *x, const QObject *y) > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue *num_y) > { > - const QNum *qnum_x = qobject_to(QNum, x); > - const QNum *qnum_y = qobject_to(QNum, y); > - const QNumValue *num_x = &qnum_x->value; > - const QNumValue *num_y = &qnum_y->value; > - > switch (num_x->kind) { > case QNUM_I64: > switch (num_y->kind) { > @@ -241,7 +236,7 @@ bool qnum_is_equal(const QObject *x, const QObject *y) > case QNUM_U64: > switch (num_y->kind) { > case QNUM_I64: > - return qnum_is_equal(y, x); > + return qnum_value_is_equal(num_y, num_x); > case QNUM_U64: > /* Comparison in native uint64_t type */ > return num_x->u.u64 == num_y->u.u64; > @@ -264,6 +259,20 @@ bool qnum_is_equal(const QObject *x, const QObject *y) > abort(); > } > > +/** > + * qnum_is_equal(): Test whether the two QNums are equal > + * @x: QNum object > + * @y: QNum object > + * > + * See qnum_value_is_equal() for details on the comparison rules. > + */ > +bool qnum_is_equal(const QObject *x, const QObject *y) > +{ > + const QNum *qnum_x = qobject_to(QNum, x); > + const QNum *qnum_y = qobject_to(QNum, y); > + return qnum_value_is_equal(&qnum_x->value, &qnum_y->value); > +} > + > /** > * qnum_destroy_obj(): Free all memory allocated by a QNum object > * > -- > 2.28.0 > > > beside that, Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
On Tue, Nov 17, 2020 at 12:42:47PM +0400, Marc-André Lureau wrote: > On Tue, Nov 17, 2020 at 2:42 AM Eduardo Habkost <ehabkost@redhat.com> wrote: > > > Extract the QNum value comparison logic to a function that takes > > QNumValue* as argument. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > include/qapi/qmp/qnum.h | 1 + > > qobject/qnum.c | 29 +++++++++++++++++++---------- > > 2 files changed, 20 insertions(+), 10 deletions(-) > > > > diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h > > index 62fbdfda68..0327ecd0f0 100644 > > --- a/include/qapi/qmp/qnum.h > > +++ b/include/qapi/qmp/qnum.h > > @@ -106,6 +106,7 @@ double qnum_get_double(const QNum *qn); > > > > char *qnum_to_string(QNum *qn); > > > > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue *num_y); > > bool qnum_is_equal(const QObject *x, const QObject *y); > > void qnum_destroy_obj(QObject *obj); > > > > diff --git a/qobject/qnum.c b/qobject/qnum.c > > index f80d4efd76..6a0f948b16 100644 > > --- a/qobject/qnum.c > > +++ b/qobject/qnum.c > > @@ -207,9 +207,9 @@ char *qnum_to_string(QNum *qn) > > } > > > > /** > > - * qnum_is_equal(): Test whether the two QNums are equal > > - * @x: QNum object > > - * @y: QNum object > > + * qnum_value_is_equal(): Test whether two QNumValues are equal > > + * @num_x: QNum value > > + * @num_y: QNum value > > > > val_x: a QNumValue ? Do you mean: @num_x: a QNumValue ? I was not planning to rename the existing num_x/num_y variables. > > * > > * Negative integers are never considered equal to unsigned integers, > > * but positive integers in the range [0, INT64_MAX] are considered > > @@ -217,13 +217,8 @@ char *qnum_to_string(QNum *qn) > > * > > * Doubles are never considered equal to integers. > > */ > > -bool qnum_is_equal(const QObject *x, const QObject *y) > > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue *num_y) > > { > > - const QNum *qnum_x = qobject_to(QNum, x); > > - const QNum *qnum_y = qobject_to(QNum, y); > > - const QNumValue *num_x = &qnum_x->value; > > - const QNumValue *num_y = &qnum_y->value; > > - > > switch (num_x->kind) { > > case QNUM_I64: > > switch (num_y->kind) { > > @@ -241,7 +236,7 @@ bool qnum_is_equal(const QObject *x, const QObject *y) > > case QNUM_U64: > > switch (num_y->kind) { > > case QNUM_I64: > > - return qnum_is_equal(y, x); > > + return qnum_value_is_equal(num_y, num_x); > > case QNUM_U64: > > /* Comparison in native uint64_t type */ > > return num_x->u.u64 == num_y->u.u64; > > @@ -264,6 +259,20 @@ bool qnum_is_equal(const QObject *x, const QObject *y) > > abort(); > > } > > > > +/** > > + * qnum_is_equal(): Test whether the two QNums are equal > > + * @x: QNum object > > + * @y: QNum object > > + * > > + * See qnum_value_is_equal() for details on the comparison rules. > > + */ > > +bool qnum_is_equal(const QObject *x, const QObject *y) > > +{ > > + const QNum *qnum_x = qobject_to(QNum, x); > > + const QNum *qnum_y = qobject_to(QNum, y); > > + return qnum_value_is_equal(&qnum_x->value, &qnum_y->value); > > +} > > + > > /** > > * qnum_destroy_obj(): Free all memory allocated by a QNum object > > * > > -- > > 2.28.0 > > > > > > > beside that, > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Thanks!
On Tue, Nov 17, 2020 at 7:49 PM Eduardo Habkost <ehabkost@redhat.com> wrote: > On Tue, Nov 17, 2020 at 12:42:47PM +0400, Marc-André Lureau wrote: > > On Tue, Nov 17, 2020 at 2:42 AM Eduardo Habkost <ehabkost@redhat.com> > wrote: > > > > > Extract the QNum value comparison logic to a function that takes > > > QNumValue* as argument. > > > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > --- > > > include/qapi/qmp/qnum.h | 1 + > > > qobject/qnum.c | 29 +++++++++++++++++++---------- > > > 2 files changed, 20 insertions(+), 10 deletions(-) > > > > > > diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h > > > index 62fbdfda68..0327ecd0f0 100644 > > > --- a/include/qapi/qmp/qnum.h > > > +++ b/include/qapi/qmp/qnum.h > > > @@ -106,6 +106,7 @@ double qnum_get_double(const QNum *qn); > > > > > > char *qnum_to_string(QNum *qn); > > > > > > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue > *num_y); > > > bool qnum_is_equal(const QObject *x, const QObject *y); > > > void qnum_destroy_obj(QObject *obj); > > > > > > diff --git a/qobject/qnum.c b/qobject/qnum.c > > > index f80d4efd76..6a0f948b16 100644 > > > --- a/qobject/qnum.c > > > +++ b/qobject/qnum.c > > > @@ -207,9 +207,9 @@ char *qnum_to_string(QNum *qn) > > > } > > > > > > /** > > > - * qnum_is_equal(): Test whether the two QNums are equal > > > - * @x: QNum object > > > - * @y: QNum object > > > + * qnum_value_is_equal(): Test whether two QNumValues are equal > > > + * @num_x: QNum value > > > + * @num_y: QNum value > > > > > > > val_x: a QNumValue ? > > Do you mean: > @num_x: a QNumValue > ? > > I was not planning to rename the existing num_x/num_y variables. > > Not renaming because that would make some churn? But this is already quite confusing, so it's better to use "val" for QNumVal and "num" for QNum I guess. If you don't want to rename it in the code, may I suggest doing it at the declaration side? Not sure it's a better idea. > > > > * > > > * Negative integers are never considered equal to unsigned integers, > > > * but positive integers in the range [0, INT64_MAX] are considered > > > @@ -217,13 +217,8 @@ char *qnum_to_string(QNum *qn) > > > * > > > * Doubles are never considered equal to integers. > > > */ > > > -bool qnum_is_equal(const QObject *x, const QObject *y) > > > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue > *num_y) > > > { > > > - const QNum *qnum_x = qobject_to(QNum, x); > > > - const QNum *qnum_y = qobject_to(QNum, y); > > > - const QNumValue *num_x = &qnum_x->value; > > > - const QNumValue *num_y = &qnum_y->value; > > > - > > > switch (num_x->kind) { > > > case QNUM_I64: > > > switch (num_y->kind) { > > > @@ -241,7 +236,7 @@ bool qnum_is_equal(const QObject *x, const QObject > *y) > > > case QNUM_U64: > > > switch (num_y->kind) { > > > case QNUM_I64: > > > - return qnum_is_equal(y, x); > > > + return qnum_value_is_equal(num_y, num_x); > > > case QNUM_U64: > > > /* Comparison in native uint64_t type */ > > > return num_x->u.u64 == num_y->u.u64; > > > @@ -264,6 +259,20 @@ bool qnum_is_equal(const QObject *x, const > QObject *y) > > > abort(); > > > } > > > > > > +/** > > > + * qnum_is_equal(): Test whether the two QNums are equal > > > + * @x: QNum object > > > + * @y: QNum object > > > + * > > > + * See qnum_value_is_equal() for details on the comparison rules. > > > + */ > > > +bool qnum_is_equal(const QObject *x, const QObject *y) > > > +{ > > > + const QNum *qnum_x = qobject_to(QNum, x); > > > + const QNum *qnum_y = qobject_to(QNum, y); > > > + return qnum_value_is_equal(&qnum_x->value, &qnum_y->value); > > > +} > > > + > > > /** > > > * qnum_destroy_obj(): Free all memory allocated by a QNum object > > > * > > > -- > > > 2.28.0 > > > > > > > > > > > beside that, > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Thanks! > > -- > Eduardo > >
On Tue, Nov 17, 2020 at 08:53:19PM +0400, Marc-André Lureau wrote: > On Tue, Nov 17, 2020 at 7:49 PM Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Tue, Nov 17, 2020 at 12:42:47PM +0400, Marc-André Lureau wrote: > > > On Tue, Nov 17, 2020 at 2:42 AM Eduardo Habkost <ehabkost@redhat.com> > > wrote: > > > > > > > Extract the QNum value comparison logic to a function that takes > > > > QNumValue* as argument. > > > > > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > > --- > > > > include/qapi/qmp/qnum.h | 1 + > > > > qobject/qnum.c | 29 +++++++++++++++++++---------- > > > > 2 files changed, 20 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h > > > > index 62fbdfda68..0327ecd0f0 100644 > > > > --- a/include/qapi/qmp/qnum.h > > > > +++ b/include/qapi/qmp/qnum.h > > > > @@ -106,6 +106,7 @@ double qnum_get_double(const QNum *qn); > > > > > > > > char *qnum_to_string(QNum *qn); > > > > > > > > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue > > *num_y); > > > > bool qnum_is_equal(const QObject *x, const QObject *y); > > > > void qnum_destroy_obj(QObject *obj); > > > > > > > > diff --git a/qobject/qnum.c b/qobject/qnum.c > > > > index f80d4efd76..6a0f948b16 100644 > > > > --- a/qobject/qnum.c > > > > +++ b/qobject/qnum.c > > > > @@ -207,9 +207,9 @@ char *qnum_to_string(QNum *qn) > > > > } > > > > > > > > /** > > > > - * qnum_is_equal(): Test whether the two QNums are equal > > > > - * @x: QNum object > > > > - * @y: QNum object > > > > + * qnum_value_is_equal(): Test whether two QNumValues are equal > > > > + * @num_x: QNum value > > > > + * @num_y: QNum value > > > > > > > > > > val_x: a QNumValue ? > > > > Do you mean: > > @num_x: a QNumValue > > > ? > > > > I was not planning to rename the existing num_x/num_y variables. > > > > > Not renaming because that would make some churn? But this is already quite > confusing, so it's better to use "val" for QNumVal and "num" for QNum I > guess. > > If you don't want to rename it in the code, may I suggest doing it at the > declaration side? Not sure it's a better idea. Yeah, I was not renaming them just to avoid churn. However, I'm already replacing `qn` variables with `qv` at patch 3/8. Replacing num_x/num_y with val_x/val_y at qnum_is_equal() (at patch 3/8 as well) wouldn't hurt too much.
Eduardo Habkost <ehabkost@redhat.com> writes: > Extract the QNum value comparison logic to a function that takes > QNumValue* as argument. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > include/qapi/qmp/qnum.h | 1 + > qobject/qnum.c | 29 +++++++++++++++++++---------- > 2 files changed, 20 insertions(+), 10 deletions(-) > > diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h > index 62fbdfda68..0327ecd0f0 100644 > --- a/include/qapi/qmp/qnum.h > +++ b/include/qapi/qmp/qnum.h > @@ -106,6 +106,7 @@ double qnum_get_double(const QNum *qn); > > char *qnum_to_string(QNum *qn); > > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue *num_y); > bool qnum_is_equal(const QObject *x, const QObject *y); > void qnum_destroy_obj(QObject *obj); > > diff --git a/qobject/qnum.c b/qobject/qnum.c > index f80d4efd76..6a0f948b16 100644 > --- a/qobject/qnum.c > +++ b/qobject/qnum.c > @@ -207,9 +207,9 @@ char *qnum_to_string(QNum *qn) > } > > /** > - * qnum_is_equal(): Test whether the two QNums are equal > - * @x: QNum object > - * @y: QNum object > + * qnum_value_is_equal(): Test whether two QNumValues are equal > + * @num_x: QNum value > + * @num_y: QNum value > * > * Negative integers are never considered equal to unsigned integers, > * but positive integers in the range [0, INT64_MAX] are considered > @@ -217,13 +217,8 @@ char *qnum_to_string(QNum *qn) > * > * Doubles are never considered equal to integers. > */ > -bool qnum_is_equal(const QObject *x, const QObject *y) > +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue *num_y) > { > - const QNum *qnum_x = qobject_to(QNum, x); > - const QNum *qnum_y = qobject_to(QNum, y); > - const QNumValue *num_x = &qnum_x->value; > - const QNumValue *num_y = &qnum_y->value; > - > switch (num_x->kind) { > case QNUM_I64: > switch (num_y->kind) { > @@ -241,7 +236,7 @@ bool qnum_is_equal(const QObject *x, const QObject *y) > case QNUM_U64: > switch (num_y->kind) { > case QNUM_I64: > - return qnum_is_equal(y, x); > + return qnum_value_is_equal(num_y, num_x); > case QNUM_U64: > /* Comparison in native uint64_t type */ > return num_x->u.u64 == num_y->u.u64; > @@ -264,6 +259,20 @@ bool qnum_is_equal(const QObject *x, const QObject *y) > abort(); > } > > +/** > + * qnum_is_equal(): Test whether the two QNums are equal > + * @x: QNum object > + * @y: QNum object > + * > + * See qnum_value_is_equal() for details on the comparison rules. > + */ > +bool qnum_is_equal(const QObject *x, const QObject *y) > +{ > + const QNum *qnum_x = qobject_to(QNum, x); > + const QNum *qnum_y = qobject_to(QNum, y); Humor me: blank line between declarations and statements, please. > + return qnum_value_is_equal(&qnum_x->value, &qnum_y->value); > +} > + > /** > * qnum_destroy_obj(): Free all memory allocated by a QNum object > *
On Thu, Nov 19, 2020 at 11:27:40AM +0100, Markus Armbruster wrote: [...] > > +bool qnum_is_equal(const QObject *x, const QObject *y) > > +{ > > + const QNum *qnum_x = qobject_to(QNum, x); > > + const QNum *qnum_y = qobject_to(QNum, y); > > Humor me: blank line between declarations and statements, please. I can do it. But why do you prefer it that way? > > > + return qnum_value_is_equal(&qnum_x->value, &qnum_y->value); > > +} > > + > > /** > > * qnum_destroy_obj(): Free all memory allocated by a QNum object > > *
Eduardo Habkost <ehabkost@redhat.com> writes: > On Thu, Nov 19, 2020 at 11:27:40AM +0100, Markus Armbruster wrote: > [...] >> > +bool qnum_is_equal(const QObject *x, const QObject *y) >> > +{ >> > + const QNum *qnum_x = qobject_to(QNum, x); >> > + const QNum *qnum_y = qobject_to(QNum, y); >> >> Humor me: blank line between declarations and statements, please. > > I can do it. But why do you prefer it that way? Habit borne out of C lacking other visual cues to distinguish declarations and statements. Declaration or statement? Tell me quick, don't analyze! mumble(*mutter)(); This "obviously" declares @mutter as pointer to function returning mumble. Except when @mumble isn't a typedef name, but a function taking one argument and returning a function that takes no argument. Then it passes *mutter to mumble(), and calls its return value. The whole point of coding style is to help readers along. Two stylistic conventions that can help here: 1. In a function call, no space between the expression denoting the called function and the (parenthesized) argument list. Elsewhere, space. So, when the example above is indeed a declaration, write it as mumble (*mutter)(); If it's function calls, write it as mumble(*mutter)(); 2. Separate declarations from statements with a blank line. Do not mix them. [...]
On Fri, Nov 20, 2020 at 07:52:31AM +0100, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > On Thu, Nov 19, 2020 at 11:27:40AM +0100, Markus Armbruster wrote: > > [...] > >> > +bool qnum_is_equal(const QObject *x, const QObject *y) > >> > +{ > >> > + const QNum *qnum_x = qobject_to(QNum, x); > >> > + const QNum *qnum_y = qobject_to(QNum, y); > >> > >> Humor me: blank line between declarations and statements, please. > > > > I can do it. But why do you prefer it that way? > > Habit borne out of C lacking other visual cues to distinguish > declarations and statements. Why is the distinction important, when many variable declarations also include initializer expressions that can be as complex as other statements? (The qobject_to() calls above are an example). > > Declaration or statement? Tell me quick, don't analyze! > > mumble(*mutter)(); > > This "obviously" declares @mutter as pointer to function returning > mumble. > > Except when @mumble isn't a typedef name, but a function taking one > argument and returning a function that takes no argument. Then it > passes *mutter to mumble(), and calls its return value. > > The whole point of coding style is to help readers along. Two stylistic > conventions that can help here: > > 1. In a function call, no space between the expression denoting the > called function and the (parenthesized) argument list. Elsewhere, > space. > > So, when the example above is indeed a declaration, write it as > > mumble (*mutter)(); > > If it's function calls, write it as > > mumble(*mutter)(); This makes lots of sense. Starting with a word followed by space is what makes declarations visually distinguishable. > > 2. Separate declarations from statements with a blank line. Do not mix > them. I'm not sure about this one, and I'm actually glad it is not part of CODING_STYLE. :) (I'll still follow your advice as maintainer of that piece of code, of course)
Eduardo Habkost <ehabkost@redhat.com> writes: > On Fri, Nov 20, 2020 at 07:52:31AM +0100, Markus Armbruster wrote: >> Eduardo Habkost <ehabkost@redhat.com> writes: >> >> > On Thu, Nov 19, 2020 at 11:27:40AM +0100, Markus Armbruster wrote: >> > [...] >> >> > +bool qnum_is_equal(const QObject *x, const QObject *y) >> >> > +{ >> >> > + const QNum *qnum_x = qobject_to(QNum, x); >> >> > + const QNum *qnum_y = qobject_to(QNum, y); >> >> >> >> Humor me: blank line between declarations and statements, please. >> > >> > I can do it. But why do you prefer it that way? >> >> Habit borne out of C lacking other visual cues to distinguish >> declarations and statements. > > Why is the distinction important, when many variable declarations > also include initializer expressions that can be as complex as > other statements? > > (The qobject_to() calls above are an example). We read left to right, and we're not good at backtracking. The earlier I know I'm reading a declaration, the better. >> Declaration or statement? Tell me quick, don't analyze! >> >> mumble(*mutter)(); >> >> This "obviously" declares @mutter as pointer to function returning >> mumble. >> >> Except when @mumble isn't a typedef name, but a function taking one >> argument and returning a function that takes no argument. Then it >> passes *mutter to mumble(), and calls its return value. >> >> The whole point of coding style is to help readers along. Two stylistic >> conventions that can help here: >> >> 1. In a function call, no space between the expression denoting the >> called function and the (parenthesized) argument list. Elsewhere, >> space. >> >> So, when the example above is indeed a declaration, write it as >> >> mumble (*mutter)(); >> >> If it's function calls, write it as >> >> mumble(*mutter)(); > > This makes lots of sense. Starting with a word followed by space > is what makes declarations visually distinguishable. Declarations need not match that pattern. Also, it's a rather subtle cue. >> 2. Separate declarations from statements with a blank line. Do not mix >> them. > > I'm not sure about this one, and I'm actually glad it is not part > of CODING_STYLE. :) That's why I ask to "humor me" :) For what it's worth, the convention is common enough to be supported by the traditional BSD indent program. > (I'll still follow your advice as maintainer of that piece of > code, of course) Thanks!
diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h index 62fbdfda68..0327ecd0f0 100644 --- a/include/qapi/qmp/qnum.h +++ b/include/qapi/qmp/qnum.h @@ -106,6 +106,7 @@ double qnum_get_double(const QNum *qn); char *qnum_to_string(QNum *qn); +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue *num_y); bool qnum_is_equal(const QObject *x, const QObject *y); void qnum_destroy_obj(QObject *obj); diff --git a/qobject/qnum.c b/qobject/qnum.c index f80d4efd76..6a0f948b16 100644 --- a/qobject/qnum.c +++ b/qobject/qnum.c @@ -207,9 +207,9 @@ char *qnum_to_string(QNum *qn) } /** - * qnum_is_equal(): Test whether the two QNums are equal - * @x: QNum object - * @y: QNum object + * qnum_value_is_equal(): Test whether two QNumValues are equal + * @num_x: QNum value + * @num_y: QNum value * * Negative integers are never considered equal to unsigned integers, * but positive integers in the range [0, INT64_MAX] are considered @@ -217,13 +217,8 @@ char *qnum_to_string(QNum *qn) * * Doubles are never considered equal to integers. */ -bool qnum_is_equal(const QObject *x, const QObject *y) +bool qnum_value_is_equal(const QNumValue *num_x, const QNumValue *num_y) { - const QNum *qnum_x = qobject_to(QNum, x); - const QNum *qnum_y = qobject_to(QNum, y); - const QNumValue *num_x = &qnum_x->value; - const QNumValue *num_y = &qnum_y->value; - switch (num_x->kind) { case QNUM_I64: switch (num_y->kind) { @@ -241,7 +236,7 @@ bool qnum_is_equal(const QObject *x, const QObject *y) case QNUM_U64: switch (num_y->kind) { case QNUM_I64: - return qnum_is_equal(y, x); + return qnum_value_is_equal(num_y, num_x); case QNUM_U64: /* Comparison in native uint64_t type */ return num_x->u.u64 == num_y->u.u64; @@ -264,6 +259,20 @@ bool qnum_is_equal(const QObject *x, const QObject *y) abort(); } +/** + * qnum_is_equal(): Test whether the two QNums are equal + * @x: QNum object + * @y: QNum object + * + * See qnum_value_is_equal() for details on the comparison rules. + */ +bool qnum_is_equal(const QObject *x, const QObject *y) +{ + const QNum *qnum_x = qobject_to(QNum, x); + const QNum *qnum_y = qobject_to(QNum, y); + return qnum_value_is_equal(&qnum_x->value, &qnum_y->value); +} + /** * qnum_destroy_obj(): Free all memory allocated by a QNum object *
Extract the QNum value comparison logic to a function that takes QNumValue* as argument. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/qapi/qmp/qnum.h | 1 + qobject/qnum.c | 29 +++++++++++++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-)