Re: [PATCH] remove squid-old.h

On 08/14/2012 05:25 AM, Amos Jeffries wrote:
> On 14/08/2012 10:12 p.m., Kinkie wrote:
>>> This branch won't pass a sourcemaintenance.sh run ...
>>> * Please run that script to find .h and .cci files which now contain
>>> squid.h - they should never include it.
>> Done. Expectedly, nothing broke from the removal.
>>
>>> * Please also check for files like src/ClientDelayConfig.cc and like
>>> src/DelayId.cc and src/DelaySpec.cc - which now include squid.h multiple
>>> times.
>> Fixed.
>>
>>> src/DescriptorSet.cc:
>>> please do not add comments on the same line as the #include. It is NOT
>>> accepted by all precompilers. Use a line before the #include.
>>> there are mix of C-style and C++-style comments being added in this
>>> way by
>>> this branch.
>> Ok, cleaning up. There's a few of those scattered elsewhere as well,
>> I'm fixing those as well.
>> Generally speaking, those comments are not really meaningful. Once
>> protos.h, typedefs.h and maybe structs.h are split into their
>> components, then it will be the right time to review and eliminate.
>>
>>> src/HttpRequest.h:
>>> - you fixed the dependency with pinnedConnection - can the un-ordering
>>> shuffle of these #include lines be reverted to the sorted order now?
>> It was already done.
>>
>>> src/auth/Acl.cc:
>>> - why is old copyright blurb being added to src/auth/Acl.* ?
>> No idea. Removed. But we should define a policy on a standard preface
>> to .h and .cc files, if we don't have one.
>>
>>> src/client_side.cc:
>>> - strangely moves the macro define of comm_close() into the system
>>> headers
>>> list.
>> Moved entirely after headers, I'm not sure it's the proper place though.
>>
>>> src/protos.h:
>>> - some of the SQUIDCEXTERN removals are leaving local-scope
>>> defines. ie
>>> they are missing the "extern" entirely after this patch. Is this good?
>> It works, but it's not really intended. Restoring.
>>
>>> src/store_digest.cc:
>>> - store_digest registration is being erased. Why? and if thats okay
>>> why not
>>> erase the now empty static function too?
>> It's not intended. Restoring.
>>
>>> There is a bit of branch cruft in the patch:
>>> * removed file 'include/leakcheck.h'
>>> * added file 'include/leakcheck.h'
>>> * added file 'src/fs/ufs/RebuildState.cc.moved'
>>> * added file 'src/fs/ufs/RebuildState.h.moved'
>>> * ... plus several others - search for .moved
>> I'll make sure that these are forgotten upon merge. In the meantime,
>> I'm trying to do a by-hand patch, attached.
>>
>
> +1.

I am guessing this is the patch that broke build in numerous places. I
am surprised it was not reverted upon the first signs of trouble. Here
are some of the errors that I see:

> helper.cc: In member function ‘void Ssl::Helper::Init()’:
> helper.cc:54: error: ‘strwordtok’ was not declared in this scope
> helper.cc:55: error: ‘wordlistAdd’ was not declared in this scope
> helper.cc:68: error: ‘wordlistAdd’ was not declared in this scope
> helper.cc: In member function ‘void Ssl::Helper::Shutdown()’:
> helper.cc:81: error: ‘wordlistDestroy’ was not declared in this scope
> MessageRep.cc: In member function ‘virtual void Adaptation::Ecap::RequestLineRep::uri(const libecap::Area&)’:
> MessageRep.cc:210: error: ‘urlParse’ was not declared in this scope
> MessageRep.cc: In member function ‘virtual libecap::Area Adaptation::Ecap::RequestLineRep::uri() const’:
> MessageRep.cc:218: error: ‘urlCanonical’ was not declared in this scope
> ServiceRep.cc: In member function ‘virtual void Adaptation::Ecap::ServiceRep::finalize()’:
> ServiceRep.cc:114: error: ‘DBG_IMPORTANT’ was not declared in this scope
> ServiceRep.cc:114: error: no match for ‘operator<<’ in ‘"WARNING: configured ...

And in one of the fixes that followed:

> +/* must be before including netfilter_ipv4.h */
> +#if HAVE_LIMITS_H
> +#include <limits.h>
> +#endif
> #if LINUX_NETFILTER
> #include <linux/netfilter_ipv4.h>
> #endif

If limits.h is needed for linux/netfilter_ipv4.h only, it should be
included inside LINUX_NETFILTER clause.

Thank you,

Alex.
Received on Wed Aug 15 2012 - 20:51:44 MDT