On Mon, Jul 09, 2018 at 11:11:30AM +0200, Emanuele Giuseppe Esposito wrote:
> +/* Graph Edge.*/
> +struct QOSGraphEdge {
> + QOSEdgeType type;
> + char *dest;
> + char *arg; /* just for CONTAIS and CONSUMED_BY */
CONTAINS?
> +/**
> + * remove_node(): removes a node @val from the nodes hash table.
> + * Note that node->name is not free'd since it will represent the
> + * hash table key
> + */
> +static void remove_node(void *val)
This is a confusing function name and doc comment. It does not remove
the node from a hash table, it just frees it.
Please call this destroy_node(). The g_hash_table_new_full() argument
is GDestroyNotify value_destroy_func, so that would be consistent with
glib naming too.
> +{
> + QOSGraphNode *node = (QOSGraphNode *) val;
There is no need to cast void pointers in C. Just "QOSGraphNode *node =
val;" will do.
> +/**
> + * build_driver_cmd_line(): builds the command line for the driver
> + * @node. The node name must be a valid qemu identifier, since it
> + * will be used to build the command line.
> + *
> + * It is also possible to pass an optional @args that will be
> + * concatenated to the command line.
> + *
> + * For drivers, prepend -device to the driver name.
> + */
> +static void build_driver_cmd_line(QOSGraphNode *node, const char *args)
Why is this called "driver" instead of "device"?
> +{
> + if (args) {
> + node->command_line = g_strconcat("-device ", node->name, ",",
> + args, NULL);
> + } else {
> + node->command_line = g_strconcat("-device ", node->name, NULL);
> + }
> +}
> +
> +/**
> + * build_test_cmd_line(): builds the command line for the test
> + * @node. The node name need not to be a valid qemu identifier, since it
> + * will not be used to build the command line.
> + *
> + * It is also possible to pass an optional @args that will be
> + * used as additional command line.
> + */
> +static void build_test_cmd_line(QOSGraphNode *node, const char *args)
> +{
> + if (args) {
> + node->command_line = g_strdup(args);
> + } else {
> + node->command_line = NULL;
> + }
This is equivalent to:
node->command_line = g_strdup(args);
https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html#g-strdup
> diff --git a/tests/libqos/qgraph.h b/tests/libqos/qgraph.h
> new file mode 100644
> index 0000000000..54a1786c1e
> --- /dev/null
> +++ b/tests/libqos/qgraph.h
> @@ -0,0 +1,259 @@
> +/*
> + * libqos driver framework
The per-function doc comment are good. Please also include higher-level
examples of how to use this. At first glance all these functions are
overwhelming and it's not clear how to implement new tests, drivers,
interfaces, or machines.
See include/qom/object.h for inspiration.
> +/* edge types*/
> +enum QOSEdgeType {
> + CONTAINS,
> + PRODUCES,
> + CONSUMED_BY
> +};
> +
> +/* node types*/
> +enum QOSNodeType {
> + MACHINE,
> + DRIVER,
> + INTERFACE,
> + TEST
> +};
The QOSEdgeType and QOSNodeType enum constants use commonly-used names
in the global namespace. Please prefix them since this is a header
file that will be included from many source files.
> +
> +/**
> + * Each driver, test or machine will have this as first field.
> + * Depending on the edge, the node will call the corresponding
> + * function when walking the path.
> + *
> + * QOSGraphObject also provides a destructor, used to deallocate the
> + * after the test has been executed.
> + */
> +struct QOSGraphObject {
> + /* for produces, returns void * */
> + QOSGetDriver get_driver;
Unused?
> + /* for contains, returns a QOSGraphObject * */
> + QOSGetDevice get_device;
Unused?
> diff --git a/tests/libqos/qgraph_extra.h b/tests/libqos/qgraph_extra.h
> new file mode 100644
> index 0000000000..22850c0368
> --- /dev/null
> +++ b/tests/libqos/qgraph_extra.h
> @@ -0,0 +1,155 @@
> +/*
> + * libqos driver framework
> + *
> + * Copyright (c) 2018 Emanuele Giuseppe Esposito
> <e.emanuelegiuse...@gmail.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License version 2 as published by the Free Software Foundation.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see
> <http://www.gnu.org/licenses/>
> + */
> +
> +#ifndef QGRAPH_EXTRA_H
> +#define QGRAPH_EXTRA_H
> +
> +#include "qgraph.h"
> +#define PRINT_DEBUG 0
I would expect the #ifdef to be in the C file that uses it. PRINT_DEBUG
is too generic a name for a .h file, it may cause namespace collisions.
signature.asc
Description: PGP signature
