Brad Hall wrote:
> On Fri, Jul 11, 2008 at 02:58:05PM -0700, Brock Pytlik wrote:
>   
>> Brock Pytlik wrote:
>>     
>>> Is it possible to catch the exception more specifically? I'd feel better 
>>> catching the ReadError specifically rather than any and all exceptions.
>>>
>>> Also, one nit, please make the comment "are no files" or "is no file".
>>>
>>> On a larger scale, I'm not intimately familiar with this code, but I 
>>> can't figure out why we're retrieving a file by hash when the only 
>>> action is a depend action. From what I can tell from the action code, 
>>> dependency actions shouldn't have a hash attribute at all. So I'm 
>>> confused why we're even touching this code with the examples given.
>>>
>>> I'm probably just having a brain freeze, but can you help me understand 
>>> what's going on here?
>>>
>>>  
>>>       
>> Ok, after poking around the code more, wouldn't the easiest thing to do 
>> be to just check whether hashes (in fetch_files_byhash) or 
>> content_hashes (in main_func) have a length of at least 1 before trying 
>> to open the tarfile? Really, can't the entire call to fetch_files_byhash 
>> be skipped if content_hashes is empty, and wouldn't that fix this 
>> problem without needing the try catch block at all?
>>
>> Brock
>>     
>
> OK, Brock is right.  The new webrev only calls fetch_files_byhash if
> content_hashes has a length > 0.  
>
>   

This looks good to me.
Thanks for getting this in Brad.

Brock
> Here's the link again: http://cr.opensolaris.org/~bgh/bug-2104/
>
> Thanks,
> Brad
>   

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to