On Mon, Jul 23, 2012 at 1:45 PM, Laszlo Ersek <ler...@redhat.com> 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