On Tue, Feb 03, 2009 at 04:06:18PM +0100, Jim Meyering wrote:
>> - err_delbr1:
>> + err_delbr1:;
>> + char ebuf[1024];
>
> A stray ';' crept in there.
Actually, it's required by C.
The ";" adds an empty statement before the new declaration,
to serve as a target for the label.
That's pretty horrible :-( I'd prefer it to just move the
ebuf inside the following if() {} block where its used, or
to the other var decls at the start of the function.
>> @@ -788,29 +789,30 @@ static int
umlStartVMDaemon(virConnectPtr conn,
>> return -1;
>> }
>>
>> + char ebuf[1024];
>> tmp = progenv;
>> while (*tmp) {
>
> Here, the var decl should be inside the while() loop too
Well, if I move the decl into that first while loop, then I'd have
to add 3 more, one into the next while loop, and then two more,
each into currently-single-stmt if-blocks. Seems slightly better
to add just that one decl than to add 4.
If its used in lots of places in the function, then its clearer to
declare at the start of the function.
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|