Re: [Qemu-devel] [PATCH 01/16] net: Add a hub net client

[Stefan Hajnoczi]

On Mon, Jul 23, 2012 at 1:45 PM, Laszlo Ersek <address@hidden> wrote:
> Two hairs to split:
>
> On 07/20/12 14:01, Stefan Hajnoczi wrote:
>
>> +static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
>> +{
>> +    VLANClientState *nc;
>> +    NetHubPort *port;
>> +    unsigned int id = hub->num_ports++;
>
> There are projects that don't like to put logic or externally visible
> side-effects into initializers. I don't know about qemu.

I see what you're saying, we also add it to the hub's port list
further down.  It could be split into _new() and hub_add_port(hub,
port) but then autogenerating a unique name becomes harder.  Since
this function is static, the scope is limited and we can assume
callers understand what is going on here.

I'd like to leave it this way or do you see a concrete change that
improves the code?

>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index bc55ed2..6618eb5 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2094,6 +2094,19 @@
>>      '*helper': 'str' } }
>>
>>  ##
>> +# @NetdevHubPortOptions
>> +#
>> +# Connect two or more net clients through a software hub.
>> +#
>> +# @hubid: hub identifier number
>> +#
>> +# Since 1.2
>> +##
>> +{ 'type': 'NetdevHubPortOptions',
>> +  'data': {
>> +    'hubid':     'int' } }
>
> I think this should say 'uint32'.

Okay.

Stefan