cvs commit: ALFS/nALFS/src/handlers remove.c

Jamie Bennett jamie at linuxfromscratch.org
Fri Jan 30 13:03:23 PST 2004


Kevin P. Fleming wrote:

> jamie at linuxfromscratch.org wrote:
>
>>   +            if ((status = execute_command("rm -fr %s", name))) {
>>   +                    Nprint_h_err("Removing failed.");
>>   +                    status = -1;
>>   +                    break;
>>   +            }
>>   +
>>   +        xfree(name);
>>   +        }
>>   +
>>   +    xfree(name);
>
>
> Looks nice Jamie, but I'd suggest adding a comment above this last 
> xfree(name) documenting that it's necessary because the loop above 
> might stop due to the break statement before the xfree(name) inside 
> the loop can be executed. Granted anyone reading the code should be 
> able to figure that out, but since it's not obvious why the line is 
> duplicated we can help them a little :-)
>
I've been toying with what to do with the exiting of this function for 
quite some
time today. I understand your stance on goto's but to me it still seems 
wrong to
use them. As K&R said in their infamous book, 'everywhere a goto is used 
can be
replaced with a non-goto snippet of code' (guessing the wording as I 
read it quite
some hours ago).  I think a setting of status and a test at the end 
fulfulls what I
am trying to achieve. If your ok with this I will use a similar style 
with <download>
and <unpack> although these have goto's in them which you would want to 
keep/not
keep?

...
Jamie Bennett      -    jamie at linuxfromscratch.org




More information about the alfs-log mailing list