| Message ID | 1250723280-3509-3-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On 08/20/2009 02:07 AM, Luiz Capitulino wrote: > QInt is a high-level data type that can be used to store integers > and perform type-safe conversions. > > The following functions are available: > > - qint_from_int() Create a new QInt from an int > - qint_from_int64() Create a new QInt from an int64_t > - qint_to_int() Export QInt to int > - qint_to_uint64() Export QInt to uint64_t > - qint_to_uint32() Export QInt to uint32_t > Why aren't the conversion functions symmetrical?
On 08/20/2009 01:07 AM, Luiz Capitulino wrote: > QInt is a high-level data type that can be used to store integers > and perform type-safe conversions. > > The following functions are available: > > - qint_from_int() Create a new QInt from an int > - qint_from_int64() Create a new QInt from an int64_t > - qint_to_int() Export QInt to int > - qint_to_uint64() Export QInt to uint64_t > - qint_to_uint32() Export QInt to uint32_t > > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > --- > Makefile | 1 + > qint.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > qint.h | 20 +++++++++++++ I don't see the need for a separate header file since 99.99% of the QObject clients would include all the .h files; also, most function could be inline in qint.h (or qobject.h) given that struct QInt is not opaque. I also noticed this from 01/29 only now: +#define QType_HEAD \ + QObject base; remove the trailing semicolon to ease automatic indentation in editors. Paolo
On 08/20/2009 09:51 AM, Avi Kivity wrote: > On 08/20/2009 02:07 AM, Luiz Capitulino wrote: >> QInt is a high-level data type that can be used to store integers >> and perform type-safe conversions. >> >> The following functions are available: >> >> - qint_from_int() Create a new QInt from an int >> - qint_from_int64() Create a new QInt from an int64_t >> - qint_to_int() Export QInt to int >> - qint_to_uint64() Export QInt to uint64_t >> - qint_to_uint32() Export QInt to uint32_t > > Why aren't the conversion functions symmetrical? I guess because an uint32_t would extend directly to uint64_t. But, the solution would be to drop qint_from_int (since also an int32_t extends correctly to uint64_t [1]) and rename qint_from_int64: Paolo [1] example program to avoid getting into C standardese: #include <stdint.h> int f(uint64_t i) { printf ("%lld\n", i); } int main() { int32_t i = -1<<31; uint32_t j = 1 << 31; f(i); f(j); }
On Thu, 20 Aug 2009 10:51:48 +0300 Avi Kivity <avi@redhat.com> wrote: > On 08/20/2009 02:07 AM, Luiz Capitulino wrote: > > QInt is a high-level data type that can be used to store integers > > and perform type-safe conversions. > > > > The following functions are available: > > > > - qint_from_int() Create a new QInt from an int > > - qint_from_int64() Create a new QInt from an int64_t > > - qint_to_int() Export QInt to int > > - qint_to_uint64() Export QInt to uint64_t > > - qint_to_uint32() Export QInt to uint32_t > > > > Why aren't the conversion functions symmetrical? Are you referring to all of them? Thinking about this now, qint_from_int() is not needed, I can drop it and rename qint_from_int64(). Regarding the 'export' ones, I thought that the compiler would warn about something like: int index = qint_to_int64(qi); forcing the user to cast by hand. But testing it now, it doesn't happen. :)
On 08/20/2009 04:24 PM, Luiz Capitulino wrote: > On Thu, 20 Aug 2009 10:51:48 +0300 > Avi Kivity<avi@redhat.com> wrote: > > >> On 08/20/2009 02:07 AM, Luiz Capitulino wrote: >> >>> QInt is a high-level data type that can be used to store integers >>> and perform type-safe conversions. >>> >>> The following functions are available: >>> >>> - qint_from_int() Create a new QInt from an int >>> - qint_from_int64() Create a new QInt from an int64_t >>> - qint_to_int() Export QInt to int >>> - qint_to_uint64() Export QInt to uint64_t >>> - qint_to_uint32() Export QInt to uint32_t >>> >>> >> Why aren't the conversion functions symmetrical? >> > Are you referring to all of them? > You're converting from signed ints, but back into unsigned ints.
On Thu, 20 Aug 2009 10:37:16 +0200 Paolo Bonzini <bonzini@gnu.org> wrote: > On 08/20/2009 01:07 AM, Luiz Capitulino wrote: > > QInt is a high-level data type that can be used to store integers > > and perform type-safe conversions. > > > > The following functions are available: > > > > - qint_from_int() Create a new QInt from an int > > - qint_from_int64() Create a new QInt from an int64_t > > - qint_to_int() Export QInt to int > > - qint_to_uint64() Export QInt to uint64_t > > - qint_to_uint32() Export QInt to uint32_t > > > > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > > --- > > Makefile | 1 + > > qint.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > qint.h | 20 +++++++++++++ > > I don't see the need for a separate header file since 99.99% of the > QObject clients would include all the .h files; also, most function > could be inline in qint.h (or qobject.h) given that struct QInt is not > opaque. We could have them in qemu-common.h or create a qemu-objects.h, but I would dislike merging everything in only one header file: different things in different header files is good practice. With regard to inline you are right, but I avoided it as there's always a debate on what should be inlined and what shouldn't, I really don't want to get into this. > I also noticed this from 01/29 only now: > > +#define QType_HEAD \ > + QObject base; > > remove the trailing semicolon to ease automatic indentation in editors. OK
On Thu, 20 Aug 2009 16:26:33 +0300 Avi Kivity <avi@redhat.com> wrote: > On 08/20/2009 04:24 PM, Luiz Capitulino wrote: > > On Thu, 20 Aug 2009 10:51:48 +0300 > > Avi Kivity<avi@redhat.com> wrote: > > > > > >> On 08/20/2009 02:07 AM, Luiz Capitulino wrote: > >> > >>> QInt is a high-level data type that can be used to store integers > >>> and perform type-safe conversions. > >>> > >>> The following functions are available: > >>> > >>> - qint_from_int() Create a new QInt from an int > >>> - qint_from_int64() Create a new QInt from an int64_t > >>> - qint_to_int() Export QInt to int > >>> - qint_to_uint64() Export QInt to uint64_t > >>> - qint_to_uint32() Export QInt to uint32_t > >>> > >>> > >> Why aren't the conversion functions symmetrical? > >> > > Are you referring to all of them? > > > > You're converting from signed ints, but back into unsigned ints. Sometimes you are so brief that a range of problems come to my mind. :) Basically, I'm doing what the code I'm replacing does: the top level function (get_expr()) converts what was typed by the user to int64_t. Command handlers, in turn, use this in several ways: some want int, some uint32_t, some uint64_t. As I said, I think we can live only with: - qint_from_int(int64_t) - qint_to_int64(const Qint *)
On 08/20/2009 05:51 PM, Luiz Capitulino wrote: > Sometimes you are so brief that a range of problems come to my > mind. :) > Sorry about that. But if you find problems I hadn't thought of, it's a good thing :) > Basically, I'm doing what the code I'm replacing does: the top > level function (get_expr()) converts what was typed by the > user to int64_t. Command handlers, in turn, use this in several > ways: some want int, some uint32_t, some uint64_t. > > As I said, I think we can live only with: > > - qint_from_int(int64_t) > - qint_to_int64(const Qint *) > Yeah.
On Thu, 20 Aug 2009 17:55:23 +0300 Avi Kivity <avi@redhat.com> wrote: > On 08/20/2009 05:51 PM, Luiz Capitulino wrote: > > Sometimes you are so brief that a range of problems come to my > > mind. :) > > > > Sorry about that. But if you find problems I hadn't thought of, it's a > good thing :) Sure, I forgot to add this is the good side.
diff --git a/Makefile b/Makefile index f6bdf84..d604302 100644 --- a/Makefile +++ b/Makefile @@ -90,6 +90,7 @@ obj-y += buffered_file.o migration.o migration-tcp.o net.o qemu-sockets.o obj-y += qemu-char.o aio.o net-checksum.o savevm.o obj-y += msmouse.o ps2.o obj-y += qdev.o qdev-properties.o ssi.o +obj-y += qint.o obj-$(CONFIG_BRLAPI) += baum.o obj-$(CONFIG_WIN32) += tap-win32.o diff --git a/qint.c b/qint.c new file mode 100644 index 0000000..8398824 --- /dev/null +++ b/qint.c @@ -0,0 +1,92 @@ +/* + * QInt data type. + * + * Copyright (C) 2009 Red Hat Inc. + * + * Authors: + * Luiz Capitulino <lcapitulino@redhat.com> + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + */ +#include "qint.h" +#include "qobject.h" +#include "qemu-common.h" + +static const QType qint_type; + +/** + * qint_from_int64(): Create a new QInt from an int64_t + * + * Return new reference. + */ +QInt *qint_from_int64(int64_t value) +{ + QInt *qi; + + qi = qemu_malloc(sizeof(*qi)); + qi->number = value; + QOBJECT_INIT(qi, &qint_type); + + return qi; +} + +/** + * qint_from_int(): Create a new QInt from an int + * + * Return new reference. + */ +QInt *qint_from_int(int value) +{ + return qint_from_int64(value); +} + +/** + * qint_to_int(): Export QInt to int type + */ +int qint_to_int(const QInt *qi) +{ + return qi->number; +} + +/** + * qint_to_uint32(): Export QInt to uint32_t type + */ +uint32_t qint_to_uint32(const QInt *qi) +{ + return qi->number; +} + +/** + * qint_to_uint64(): Export QInt to uint64_t type + */ +uint64_t qint_to_uint64(const QInt *qi) +{ + return qi->number; +} + +/** + * qobject_to_qint(): Convert a QObject into a QInt + */ +QInt *qobject_to_qint(const QObject *obj) +{ + if (qobject_type(obj) != QTYPE_QINT) + return NULL; + + return container_of(obj, QInt, base); +} + +/** + * qint_destroy_obj(): Free all memory allocated by a + * QInt object + */ +static void qint_destroy_obj(QObject *obj) +{ + assert(obj != NULL); + qemu_free(qobject_to_qint(obj)); +} + +static const QType qint_type = { + .code = QTYPE_QINT, + .destroy = qint_destroy_obj, +}; diff --git a/qint.h b/qint.h new file mode 100644 index 0000000..1307761 --- /dev/null +++ b/qint.h @@ -0,0 +1,20 @@ +#ifndef QINT_H +#define QINT_H + +#include "qobject.h" +#include "qemu-common.h" +#include <stdint.h> + +typedef struct QInt { + QType_HEAD + int64_t number; +} QInt; + +QInt *qint_from_int(int value); +QInt *qint_from_int64(int64_t value); +int qint_to_int(const QInt *qi); +uint32_t qint_to_uint32(const QInt *qi); +uint64_t qint_to_uint64(const QInt *qi); +QInt *qobject_to_qint(const QObject *obj); + +#endif /* QINT_H */ diff --git a/qobject.h b/qobject.h index c23b5f3..8ff7bfd 100644 --- a/qobject.h +++ b/qobject.h @@ -41,6 +41,7 @@ typedef enum { QTYPE_NONE, + QTYPE_QINT, } qtype_code; struct QObject;
QInt is a high-level data type that can be used to store integers and perform type-safe conversions. The following functions are available: - qint_from_int() Create a new QInt from an int - qint_from_int64() Create a new QInt from an int64_t - qint_to_int() Export QInt to int - qint_to_uint64() Export QInt to uint64_t - qint_to_uint32() Export QInt to uint32_t Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- Makefile | 1 + qint.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qint.h | 20 +++++++++++++ qobject.h | 1 + 4 files changed, 114 insertions(+), 0 deletions(-) create mode 100644 qint.c create mode 100644 qint.h