Re: [Qemu-devel] [PATCH 5/5] tests: qgraph API for the qtest driver framework

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2673 bytes --]

On 1/18/19 10:39 AM, Thomas Huth wrote:

>> +    QOSGraphEdge *edge = g_new0(QOSGraphEdge, 1);
>> +    edge->type = type;
>> +    edge->dest = g_strdup(dest);
>> +    edge->edge_name = g_strdup(opts->edge_name ? : dest);
> 
> I think I'd write the elvis operator rather as "?:", without the space
> inbetween. (or does our checkpatch script dislike that?)

$ git grep '? :' | wc
     15     121    1059
$ git grep '?:' | wc
    270    1418   19542


>> +bool qos_graph_has_edge(const char *start, const char *dest)
>> +{
>> +    QOSGraphEdgeList *list = get_edgelist(start);
>> +    QOSGraphEdge *e = search_list_edges(list, dest);
>> +    if (e) {
>> +        return TRUE;
>> +    }
>> +    return FALSE;
>> +}
> 
> I'm surprised that TRUE and FALSE are also still available with capital
> letters these days ... The spelling from stdbool.h is lowercase.

All caps versions are from glib's gboolean type (which is NOT a good
substitute for C99 bool), and should be avoided for any use that does
not specifically require gboolean due to type compatibility.

> 
> Anyway, maybe rather:
> 
>     return e != NULL;

Or return !!e


>> +void qos_add_test(const char *name, const char *interface,
>> +                  QOSTestFunc test_func, QOSGraphTestOptions *opts)
>> +{
>> +    QOSGraphNode *node;
>> +    char *test_name = g_strdup_printf("%s-tests/%s", interface, name);;
>> +
>> +    if (!opts) {
>> +        opts = &(QOSGraphTestOptions) { };
> 
> Same as above ... is the pointer still valid after the next curly brace?

https://stackoverflow.com/questions/47691857/lifetime-of-a-compound-literal

says it is not strict C99, but that

"A far more reasonable rule would indicate that the lifetime of a
compound literal extends until code leaves the function in which it was
used, or the expression creating it is re-executed, whichever happens
first. Since the Standard allows compilers to extend the lifetime of
automatic objects however they see fit, the fact that a compiler does so
should not be considered a bug. On the other hand, a quality compiler
which is deliberately going to be more useful than the Standard requires
should probably explicitly document that fact. Otherwise future
maintainers may declare that any programs that rely upon such sensible
behavior are "defective", and that the compiler can be more "efficient"
if it ceases to support them."

I'm surprised there is not a defect against the C standard to make the
desired semantics explicit.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]