Re: [PATCH] Add a progress bar to file downloads.

[David Cantrell <dcantrell redhat com>]

On Aug 11, 2008, at 7:52 AM, Ricky Zhou wrote:

> Here's a patch to add progress bars to file downloads.  Like I
> mentioned, I'm pretty new to C, so please feel free to tear this apart
> and let me know if I'm doing anything in a non-proper way.


The patch looks nice.  New to C and you are working with libnewt...I'm  
very sorry.  :)

I have some comments having just read through the patch.  Other people  
might want to add comments:

1) Please use the same indentation style that the rest of the code is  
using.  We are not consistent in the anaconda code, but if you're  
adding something, make sure you follow the same indentation style.

2) Check the return value of malloc() and handle it appropriately.   
Again, this is not something that is consistent throughout the code,  
but it's another thing on my to do list.  'Appropriately' depends on  
whether or not error recovery is possible.  Generally no and if  
malloc() fails, it's ok to log a message and abort().  Checking for  
things like this helps finding where things are failing later on.   
People will say malloc should not fail, which is true, it shouldn't if  
you want your program to continue.  But, it can, and if it does we  
want to know about it and stop then.

3) alloca() is used a lot in our code, but it's not one of my favorite  
functions because it's not allowed to fail even though memory  
resources are finite.  I would prefer we use malloc() or calloc() just  
to gain the capability of checking for the failure on allocation.  In  
my opinion, it's worth the tradeoffs that alloca() would give us.

Other than those things, I like what I see.

--
David Cantrell <dcantrell redhat com>
Red Hat / Honolulu, HI