Hi Amos,
Amos Jeffries wrote:
> I just spent the afternoon going over Christos full patch for
> squid3-largeobj and only found a few very minor things. Though I still
> don't know squid well enough to tell if there is anything missed out.
> 
Did you use it to download large files? Did you do some tests using it
(just browsing ot something else)? I believe that now it is in good state...

> HttpHdrRange.cc chunk @459
>  - (uint64_t) looks to be obsolete cast now in the if.

And some other thinks needed small changes too...

> 
> Not sure whether these are needed, but its not clear either way without a
> compile test.
> peer_digest.cc chunk @787
>  - (int)
Needed here, else the compiler will produce a warning.....
> store_dir.cc chunk @330
>  - (uint64_t )
Where is it?

> Some formatting niggles:
> access_log.cc chunk @531
> - outoff, and dooff definitions are indented with tab, needs to be space.
> 
> src/Makefile.am chunk @584
>  - makefiles are all tab indented. StoreMetaSTDLFS.h has been spaced.
> (yes its weird, but I think its a carryover from an automake syntax
> requirement)
Done ..

> 
> 
> ufsdump.cc chunk @89
> - I would suggest doing a single static_cast in each of your cases and
> then using the converted pointer. You will need to add {}. That drops 6
> castings down to 2 and makes it more readable.
> - Also, anything streamed should not need casting unless its stored wrong
> (the "(int)x.getType()" )

Typecasting for x.getType() needed because it returns a char type but we
need to print an integer here. But ufsdump is just a debugging tool for
internal use only ....

Regards,
    Christos



Reply via email to