From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MyQuw-0004BU-Je for qemu-devel@nongnu.org; Thu, 15 Oct 2009 10:02:58 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MyQus-00048k-R7 for qemu-devel@nongnu.org; Thu, 15 Oct 2009 10:02:58 -0400 Received: from [199.232.76.173] (port=58186 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MyQus-00048b-MT for qemu-devel@nongnu.org; Thu, 15 Oct 2009 10:02:54 -0400 Received: from mail-qy0-f199.google.com ([209.85.221.199]:34257) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MyQus-0000kl-5M for qemu-devel@nongnu.org; Thu, 15 Oct 2009 10:02:54 -0400 Received: by qyk37 with SMTP id 37so600399qyk.18 for ; Thu, 15 Oct 2009 07:02:53 -0700 (PDT) Message-ID: <4AD72B88.2040107@codemonkey.ws> Date: Thu, 15 Oct 2009 09:02:48 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 01/10] Introduce qmisc module References: <1255037747-3340-1-git-send-email-lcapitulino@redhat.com> <1255037747-3340-2-git-send-email-lcapitulino@redhat.com> In-Reply-To: <1255037747-3340-2-git-send-email-lcapitulino@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel@nongnu.org Luiz Capitulino wrote: > This module provides miscellania QObject functions. > > Currently it exports qobject_from_fmt(), which is somewhat > based on Python's Py_BuildValue() function. It is capable of > creating QObjects from a specified string format. > > For example, to create a QDict with mixed data-types one > could do: > > QObject *obj = qobject_from_fmt("{ s: [ i, s ], s: i }", ... ); > > Signed-off-by: Luiz Capitulino > --- > Makefile | 2 +- > qmisc.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > qmisc.h | 19 +++++ > 3 files changed, 242 insertions(+), 1 deletions(-) > create mode 100644 qmisc.c > create mode 100644 qmisc.h > > diff --git a/Makefile b/Makefile > index d96fb4b..182f176 100644 > --- a/Makefile > +++ b/Makefile > @@ -100,7 +100,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 qstring.o qdict.o qlist.o qemu-config.o > +obj-y += qint.o qstring.o qdict.o qlist.o qmisc.o qemu-config.o > > obj-$(CONFIG_BRLAPI) += baum.o > obj-$(CONFIG_WIN32) += tap-win32.o > diff --git a/qmisc.c b/qmisc.c > new file mode 100644 > index 0000000..42b6f22 > --- /dev/null > +++ b/qmisc.c > @@ -0,0 +1,222 @@ > +/* > + * Misc QObject functions. > + * > + * Copyright (C) 2009 Red Hat Inc. > + * > + * Authors: > + * Luiz Capitulino > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + */ > +#include "qmisc.h" > +#include "qint.h" > +#include "qlist.h" > +#include "qdict.h" > +#include "qstring.h" > +#include "qobject.h" > +#include "qemu-common.h" > + > +/* > + * qobject_from_fmt() and related functions are based on the Python's > + * Py_BuildValue() and are subject to the Python Software Foundation > + * License Version 2. > + * > + * Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009 Python > + * Software Foundation. > + */ > If we're introducing third-party code under a new license, we need to update the top-level LICENSE file. I took a brief look and it wasn't immediately clear that this license is GPL compatible. According to the FSF, certain versions of this license are incompatible and some are compatible. I think it would have been better to just write something from scratch... > + case '[': > + return do_mklist(fmt, args, ']', count_format(*fmt, ']')); > Because this is bizarre. It looks ahead to count the number of arguments which is a very strange way to parse something like this. Why not a simple recursive decent parser? > +/** > + * qobject_from_fmt(): build QObjects from a specified format. > + * > + * Valid characters of the format: > + * > + * i integer, map to QInt > + * s string, map to QString > + * [] list, map to QList > + * {} dictionary, map to QDict > + * > + * Examples: > + * > + * - Create a QInt > + * > + * qobject_from_fmt("i", 42); > + * > + * - Create a QList of QStrings > + * > + * qobject_from_fmt("[ i, i, i ]", 0, 1 , 2); > + * > + * - Create a QDict with mixed data-types > + * > + * qobject_from_fmt("{ s: [ i, s ], s: i }", ... ); > + * > + * Return a strong reference to a QObject on success, NULL otherwise. > + */ > But my real objection is that we should make this "{%s: [%d, %s], %s: %d}" so that we can mark it as a printf formatted function and get type checking. You'll probably have to support both "%d" and "%" PRId64 for sanity sake. Regards, Anthony Liguori