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.
