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

[Stefan Hajnoczi]

On Mon, Jul 23, 2012 at 3:05 PM, Laszlo Ersek <address@hidden> wrote:
> On 07/23/12 15:49, Stefan Hajnoczi wrote:
>> 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?
>
> Oh no, that's not what I meant -- the function is perfectly fine, it's
> just that the post-increment of object A shouldn't be in the definition
> / initializer list of object B. (Keeping the increment and the
> construcion in one place is actually a good thing IMHO.)
>
> The idea is, rather than
>
>     unsigned int id = hub->num_ports++;
>
> it should say
>
>     unsigned int id;
>     /* ... */
>     id = hub->num_ports++;
>
> ... Hm, yes, I remember it from
> <http://www.manpages.info/freebsd/style.9.html>. But again, I'm not sure
> how qemu treats this.

"Be careful to not obfuscate the code by initializing variables in the
declarations.  Use this feature only thoughtfully.  DO NOT use
function calls in initializers."

This?

Do you know the rationale?  It's not clear to me how this "obfuscates" the code.

Messing around with side-effects in a series of variable declarations
with intializers could be bug-prone.  But here there is only one
initializer so it's not a problem.

Regarding QEMU, there's no coding style rule against initializers.
Personally I think that's a good thing because it keeps the code
concise - people reading it don't have to keep the declaration in
their mind until they hit the initializer later on.

Stefan