squid3 future directory structure

[Alex Rousskov]

On Tue, 2008-02-12 at 11:19 +1300, Amos Jeffries wrote:
> > On Tue, 2008-02-12 at 10:37 +1300, Amos Jeffries wrote:
> >> > When we get a better VCS, we should discuss moving include/ and lib/
> >> > stuff into src/ with the exception of 3rd party code. This would avoid
> >> > problems created by that artificial boundary.
> >>
> >> What I have been mulling over after seeing code heirarchy like this:
> >>
> >> /include + /lib  == C library functions for portability (autotools
> >> requires these here).
> >
> > In my experience, autotools do not require them to be there or those
> > requirements can be relaxed as needed. Our portability wrappers may
> > still use Squid-specific code so placing them outside of src/ is not a
> > good idea, IMO. There got to be a better reason to place something
> > outside of src/ than "autotools" :-).
> 
> My reading of the AC_REPLACE_FUNCS(...) is that it when it detects a
> portability issue it adds include/missing-function.h and
> lib/missing-function.c to the build list.
> We are using that for several library functions.

Does AC_CONFIG_LIBOBJ_DIR control the name of the "lib/" directory?

 Macro: AC_CONFIG_LIBOBJ_DIR (DIRECTORY)
 Specify that `AC_LIBOBJ' replacement files are to be found in
 DIRECTORY, a name relative to the top level of the source tree.
 The replacement directory defaults to `.', the top level
 directory, and the most typical value is `lib', corresponding to
 `AC_CONFIG_LIBOBJ_DIR([lib])'.

 `configure' might need to know the replacement directory for the
 following reasons: (i) some checks use the replacement files, (ii)
 some macros bypass broken system headers by installing links to the
 replacement headers (iii) when used in conjunction with Automake,
 within each makefile, DIRECTORY is used as a relative path from
 `$(top_srcdir)' to each object named in `LIBOBJS' and `LTLIBOBJS',
 etc.

> >> ?somewhere? == third party additions (currently /lib//*
> >
> > /lib/name is not too bad. We can even do src/3rdparty/name or something
> > like that, but perhaps keeping that stuff outside of src/ is a good idea
> > even though it is also "source".
> 
> I'm not disagreeing on lib/name/. Just splitting the so its kept seperate
> from the raw portability files.

Right. FWIW, the dir/many-files-here* plus dir/many-subdirs-here/*
combination always looks messy and awkward to search/navigate to me. I
would prefer just dir/many-subdirs-here/* but it is not a big deal.

> >
> >> /src/ == core code at lowest level
> >
> > I would move it to src/base or src/backend or something like that. And
> > there should not be really many files there.
> 
> /src/common ?? (I think you were against a /src/core earlier)

"Common" is good enough to me as it reflects the purpose fairly well.
"Core" would be wrong because this directory is for little things used
throughout the project rather than some kind of a monolithic kernel
holding everything together.

BTW, we should not name that subdir "core" regardless of its purpose --
I have tried that in another project and got bitten by systems/programs
that treat that name specially :-).


Another big related question is the header path problem: Do we want to
have something like squid/src/squid/module/*, which allows you to refer
to Squid include files as  as opposed to
 or, horror, ?

AFAIK, this arrangement is necessary if you:

1) want to install some of the header files from subdirs (e.g., for
folks who build 3rd party eCAP modules);

2) do not want to pollute global header namespace with likely-to-clash
file names like Log.h or select.h; and

3) do not want to keep all header files separately from .cc files, using
the squid/include/module/something.h template (which I find much uglier
and more awkward to use than squid/src/squid/module/ where related .h
and .cc files are kept in one module/ directory).

There may be better solutions to the header path problem, but I do not
know them.

HTH,

Alex.

Re: squid-3.HEAD IPAddress leak

[Robert Collins]

On Fri, 2008-02-08 at 12:14 +0900, Adrian Chadd wrote:
> 
> > Together they make a pretty tree. But every used piece is
> eseentially
> > another new, memset, free.
> 
> Ah, and here you will have problems.
> 
> The members of that struct should probably be malloc, free, and not
> new/delete. You're using new/delete which -should- map to a default
> new operator and head off to the malloc libraries, but -squids- idea
> of the malloc interface could differ from the -library- idea of
> the malloc interface.

You can tell C++ to use malloc/free if needed. Am I reading right that
the OS calls free itself ? Usually /either/ clients free and allocate,
or libraries free and allocate - never a mix. (Because even in C the
malloc libraries can be partially overridden).

If squid is doing the allocation and free the following:

> So you should probably drop the new/delete'ing of the addrinfo stuff
> and replace it with malloc/free.

is irrelevant, but if its shared then we should definitely not use C++
because operator delete can't be called. (And don't use talloc, or
anything else either.

> You're also memset()'ing the addrinfo struct whether you allocated
> it or not, which may be double-memset'ing the thing, and if someone
> passed in an addrinfo it may have structure members which have now
> been leaked.

tighter boundaries may help this.

-Rob

-- 
GPG key available at: .


signature.asc
Description: This is a digitally signed message part

Re: two xasserts in squid3

[Alex Rousskov]

On Tue, 2008-02-12 at 11:27 +1300, Amos Jeffries wrote:
> >>
> >> * Add automatic testing for header dependency
> >> - script to perform universal include unit-test for .h files
> >> - link to automatic unit-testing in each directory
> >> - fix the compile errors!
> >>

> I am thinking along the lines of unit-testing the compilation of each *.h
> file.
> 
> At present a fvery few of the important .h files have a hard-coded .cc in
> the tests directory and that gets built when unit-testing. All it does is
> "#include "
> 
> What I'm planning is a test-suite script that when passed a directory,
> scans it for all .h files. Writes the .cc and attempts a build. Any
> compile errors are dependancy errors needing a fix.
> 
> That would speed-track the dependancy testing in future with no need for
> manually adding these tests, or a large set of files for them. It is
> particularly useful for the struct and proto re-arranging.

Sounds good.

Thank you,

Alex.

Re: two xasserts in squid3

[Amos Jeffries]

>
> On Tue, 2008-02-12 at 11:10 +1300, Amos Jeffries wrote:
>> > On Mon, 2008-02-11 at 09:33 +1100, Robert Collins wrote:
>> >> One of the things I'd most love to see is the modularisation
>> completed
>> >> - complete deletion of protos.h, structs.h, typedefs.h.
>> >
>> > Yes, of course. That would be the focus of the cleanup that Amos is
>> > volunteering to do :-)
>> >
>>
>> Hows this for an action plan Robert?
>>
>> * Obsolete typedefs.h (underway)
>> - remove all unneeded typedefs
>> - move all needed typedefs to their appropriate headers
>> - fix compile errors
>>
>> * Add automatic testing for header dependency
>> - script to perform universal include unit-test for .h files
>> - link to automatic unit-testing in each directory
>> - fix the compile errors!
>>
>> * Obsolete protos.h
>> - move all protos to their appropriate header files
>> - add includes for headers where needed.
>>
>> * Obsolete structs.h
>> - move all structs to their appropriate header files
>> - move modular configuration in to *Config.h files
>>   (discussion on exactly what the modules are)
>
> I like the automatic include unit test. I found while slowly working
> through this that circular structure dependencies made it quite tricky;

Yes found that already, and have not even started. ;-)
I can't believe I'm thankful that many of them use pointer-fields.

> I think you will find you need
>
>  * Break dependencies

Please explain. I assume you mean the "class Something;" pre-definitions
instead of #includes.

>
> to make it really clean, but that can be longer term if needed.
>
> -Rob
> --
> GPG key available at: .
>

Re: two xasserts in squid3

[Robert Collins]

On Tue, 2008-02-12 at 11:10 +1300, Amos Jeffries wrote:
> > On Mon, 2008-02-11 at 09:33 +1100, Robert Collins wrote:
> >> One of the things I'd most love to see is the modularisation completed
> >> - complete deletion of protos.h, structs.h, typedefs.h.
> >
> > Yes, of course. That would be the focus of the cleanup that Amos is
> > volunteering to do :-)
> >
> 
> Hows this for an action plan Robert?
> 
> * Obsolete typedefs.h (underway)
> - remove all unneeded typedefs
> - move all needed typedefs to their appropriate headers
> - fix compile errors
> 
> * Add automatic testing for header dependency
> - script to perform universal include unit-test for .h files
> - link to automatic unit-testing in each directory
> - fix the compile errors!
> 
> * Obsolete protos.h
> - move all protos to their appropriate header files
> - add includes for headers where needed.
> 
> * Obsolete structs.h
> - move all structs to their appropriate header files
> - move modular configuration in to *Config.h files
>   (discussion on exactly what the modules are)

I like the automatic include unit test. I found while slowly working
through this that circular structure dependencies made it quite tricky;
I think you will find you need

 * Break dependencies

to make it really clean, but that can be longer term if needed.

-Rob
-- 
GPG key available at: .


signature.asc
Description: This is a digitally signed message part

Re: two xasserts in squid3

[Amos Jeffries]

>
> On Tue, 2008-02-12 at 11:10 +1300, Amos Jeffries wrote:
>
>> * Obsolete typedefs.h (underway)
>> - remove all unneeded typedefs
>> - move all needed typedefs to their appropriate headers
>> - fix compile errors
>>
>> * Add automatic testing for header dependency
>> - script to perform universal include unit-test for .h files
>> - link to automatic unit-testing in each directory
>> - fix the compile errors!
>>
>> * Obsolete protos.h
>> - move all protos to their appropriate header files
>> - add includes for headers where needed.
>>
>> * Obsolete structs.h
>> - move all structs to their appropriate header files
>> - move modular configuration in to *Config.h files
>>   (discussion on exactly what the modules are)
>
>> * Auto-doc the API for modules decided above
>>
>> * Move files into appropriate sub-dirs based on modules
>
> Amos,
>
> All sounds good, except I do not understand the "Add automatic
> testing for header dependency" blob. Can you describe that in more
> detail? What is it, and why do we need it?

I am thinking along the lines of unit-testing the compilation of each *.h
file.

At present a fvery few of the important .h files have a hard-coded .cc in
the tests directory and that gets built when unit-testing. All it does is
"#include "

What I'm planning is a test-suite script that when passed a directory,
scans it for all .h files. Writes the .cc and attempts a build. Any
compile errors are dependancy errors needing a fix.

That would speed-track the dependancy testing in future with no need for
manually adding these tests, or a large set of files for them. It is
particularly useful for the struct and proto re-arranging.

Amos

Re: two xasserts in squid3

[Amos Jeffries]

> On Tue, 2008-02-12 at 10:37 +1300, Amos Jeffries wrote:
>> > When we get a better VCS, we should discuss moving include/ and lib/
>> > stuff into src/ with the exception of 3rd party code. This would avoid
>> > problems created by that artificial boundary.
>>
>> What I have been mulling over after seeing code heirarchy like this:
>>
>> /include + /lib  == C library functions for portability (autotools
>> requires these here).
>
> In my experience, autotools do not require them to be there or those
> requirements can be relaxed as needed. Our portability wrappers may
> still use Squid-specific code so placing them outside of src/ is not a
> good idea, IMO. There got to be a better reason to place something
> outside of src/ than "autotools" :-).

My reading of the AC_REPLACE_FUNCS(...) is that it when it detects a
portability issue it adds include/missing-function.h and
lib/missing-function.c to the build list.
We are using that for several library functions.

>
>> ?somewhere? == third party additions (currently /lib//*
>
> /lib/name is not too bad. We can even do src/3rdparty/name or something
> like that, but perhaps keeping that stuff outside of src/ is a good idea
> even though it is also "source".

I'm not disagreeing on lib/name/. Just splitting the so its kept seperate
from the raw portability files.

>
>> /src/ == core code at lowest level
>
> I would move it to src/base or src/backend or something like that. And
> there should not be really many files there.

/src/common ?? (I think you were against a /src/core earlier)

>
>> /src/ == 'independant' module code. namely the files like
>> existing
>> ICAP, FS, Auth code. But making sure that all the other independant code
>> gets its own sub-dirs too (thinking each server-side gateway, pinger,
>> DNS?, ESI, Others?)
>
> Yes, but "module" and "independent" should be treated loosely. For
> example, logging may not be a "module" or "independent", but may need
> its own directory. Same for memory management.
>
> In short, when you have or want to have a bunch of Something*.{cc,h}
> files, place them into src/Something/ directory.

Exactly.

Amos

Re: two xasserts in squid3

[Alex Rousskov]

On Tue, 2008-02-12 at 11:10 +1300, Amos Jeffries wrote:

> * Obsolete typedefs.h (underway)
> - remove all unneeded typedefs
> - move all needed typedefs to their appropriate headers
> - fix compile errors
> 
> * Add automatic testing for header dependency
> - script to perform universal include unit-test for .h files
> - link to automatic unit-testing in each directory
> - fix the compile errors!
> 
> * Obsolete protos.h
> - move all protos to their appropriate header files
> - add includes for headers where needed.
> 
> * Obsolete structs.h
> - move all structs to their appropriate header files
> - move modular configuration in to *Config.h files
>   (discussion on exactly what the modules are)

> * Auto-doc the API for modules decided above
> 
> * Move files into appropriate sub-dirs based on modules

Amos,

All sounds good, except I do not understand the "Add automatic
testing for header dependency" blob. Can you describe that in more
detail? What is it, and why do we need it?

Thank you,

Alex.

Re: two xasserts in squid3

[Amos Jeffries]

> On Mon, 2008-02-11 at 09:33 +1100, Robert Collins wrote:
>> One of the things I'd most love to see is the modularisation completed
>> - complete deletion of protos.h, structs.h, typedefs.h.
>
> Yes, of course. That would be the focus of the cleanup that Amos is
> volunteering to do :-)
>

Hows this for an action plan Robert?

* Obsolete typedefs.h (underway)
- remove all unneeded typedefs
- move all needed typedefs to their appropriate headers
- fix compile errors

* Add automatic testing for header dependency
- script to perform universal include unit-test for .h files
- link to automatic unit-testing in each directory
- fix the compile errors!

* Obsolete protos.h
- move all protos to their appropriate header files
- add includes for headers where needed.

* Obsolete structs.h
- move all structs to their appropriate header files
- move modular configuration in to *Config.h files
  (discussion on exactly what the modules are)


And the long term end...

* Auto-doc the API for modules decided above

* Move files into appropriate sub-dirs based on modules


Amos

Re: two xasserts in squid3

[Alex Rousskov]

On Tue, 2008-02-12 at 10:37 +1300, Amos Jeffries wrote:
> > When we get a better VCS, we should discuss moving include/ and lib/
> > stuff into src/ with the exception of 3rd party code. This would avoid
> > problems created by that artificial boundary.
> 
> What I have been mulling over after seeing code heirarchy like this:
> 
> /include + /lib  == C library functions for portability (autotools
> requires these here).

In my experience, autotools do not require them to be there or those
requirements can be relaxed as needed. Our portability wrappers may
still use Squid-specific code so placing them outside of src/ is not a
good idea, IMO. There got to be a better reason to place something
outside of src/ than "autotools" :-).

> ?somewhere? == third party additions (currently /lib//*

/lib/name is not too bad. We can even do src/3rdparty/name or something
like that, but perhaps keeping that stuff outside of src/ is a good idea
even though it is also "source".

> /src/ == core code at lowest level

I would move it to src/base or src/backend or something like that. And
there should not be really many files there.

> /src/ == 'independant' module code. namely the files like existing
> ICAP, FS, Auth code. But making sure that all the other independant code
> gets its own sub-dirs too (thinking each server-side gateway, pinger,
> DNS?, ESI, Others?)

Yes, but "module" and "independent" should be treated loosely. For
example, logging may not be a "module" or "independent", but may need
its own directory. Same for memory management. 

In short, when you have or want to have a bunch of Something*.{cc,h}
files, place them into src/Something/ directory.

Alex.

Re: a simple formatter

[Amos Jeffries]

>
> On Fri, 2008-02-08 at 23:26 +0200, Tsantilas Christos wrote:
>> Alex Rousskov wrote:
>> >
>> > Changes in the current code are only bad for outstanding patches and
>> > forgotten branches, right? If everybody applies the same formatting to
>> > all their branches and HEAD, we should not have many conflicts, right?
>>
>> I don't know, possibly it is OK.
>> Or just format the HEAD, merge HEAD to branches (this will take some
>> hours but...) and then apply formating to the branch. After the first
>> time all will be OK.
>
> with CVS you will be much better off formatting all branches
> simultaneously, then merging.

I think that would cause clashes on any very old branches. They would need
merging up before the format then formatting HEAD & branches and
re-merging.

Amos

Re: two xasserts in squid3

[Amos Jeffries]

> On Mon, 2008-02-11 at 10:15 +1300, Amos Jeffries wrote:
>
>> I think a better approach to this would be:
>>
>> 1) do we actually need it anyway?
>
> Yes, we need a common assert-like macro.
>
>> 2) where is it supposed to be defined?
>
> IMO:
>
> include/xassert.h if xassert is used outside of src/
>
> src/xassert.h otherwise, until we get something like src/base/ or
> src/misc for general-purpose commonly used things that do not fit
> anywhere else.
>
>> I'm willing to commit time to that if we can agree on the 'old' files
>> that need removing and the scope of the remining ones.
>> I have seen a few things that made me think squid.h and defines.h may
>> need to (merge?) stick around under a better definition of content.
>
> I would welcome such cleanup, especially after we switch to a VCS that
> does file renaming.
>
> IMO, there needs to be one header file like squid.h that is guaranteed
> to be the first one included by all other source files in src/. That
> file should only contain things needed by 99.9% of the code base. This
> is usually limited to hacks that change some global system or compiler
> effects. The rest should have their own purpose-specific header files.

I was thinking that squid.h is the good choice for that. Just with much
less tangled dependancy than it currently has. With some that is currently
in defines.h.

FWIW: I have a test run going over typedefs and managed to kill a few
dozen lines of code there without side-effects on linux. I'll have it in
cleanup branch soon for testing on other OS and compilers.

>
> If the source code needs something used by others, it needs to include
> something.h.
>
> There is also a question of whether some lib/ stuff needs to be moved to
> src/, but that is a separate debate and we need a better VCS for that as
> well.

Some does, ie IPAddress I think should go back. My initial reason for
placing it there is no longer valid. IMO Mem* and Debug shouls not be
there either, but in wherever the core goes.

Amos

Re: two xasserts in squid3

[Amos Jeffries]

>
> On Sun, 2008-02-10 at 20:51 +0200, Tsantilas Christos wrote:
>> Maybe it was better if the files lib/assert.c and include/assert.h
>> removed and the assert macro  defined in squid.h file like squid2.6
>> does...
>
> It looks like Array, MemPool, and splay files are using assert outside
> of src/ so you will not be able to remove include/assert.h. However, you
> may be able to declare xassert in include/assert.h while defining it in
> src/Debug.cc.
>
> The assert #define in src/Debug.h should probably be moved to
> include/assert.h. These #defines must be the same.
>
> When we get a better VCS, we should discuss moving include/ and lib/
> stuff into src/ with the exception of 3rd party code. This would avoid
> problems created by that artificial boundary.

What I have been mulling over after seeing code heirarchy like this:

/include + /lib  == C library functions for portability (autotools
requires these here).

?somewhere? == third party additions (currently /lib//*

/src/ == core code at lowest level

/src/ == 'independant' module code. namely the files like existing
ICAP, FS, Auth code. But making sure that all the other independant code
gets its own sub-dirs too (thinking each server-side gateway, pinger,
DNS?,
ESI, Others?)


Have I understood the earlier plans right? I think its a good structure,
but as you say needs a better VCS to implement fully.

Amos

Re: a simple formatter

[Robert Collins]

On Fri, 2008-02-08 at 23:26 +0200, Tsantilas Christos wrote:
> Alex Rousskov wrote:
> > 
> > Changes in the current code are only bad for outstanding patches and
> > forgotten branches, right? If everybody applies the same formatting to
> > all their branches and HEAD, we should not have many conflicts, right?
> 
> I don't know, possibly it is OK.
> Or just format the HEAD, merge HEAD to branches (this will take some
> hours but...) and then apply formating to the branch. After the first
> time all will be OK.

with CVS you will be much better off formatting all branches
simultaneously, then merging.

-Rob
-- 
GPG key available at: .


signature.asc
Description: This is a digitally signed message part

Re: X-Vary-Options patch

[Robert Collins]

On Fri, 2008-02-08 at 16:26 +1100, Tim Starling wrote:
> 
> 
> The added features of the patch are conditional, and are enabled by
> the 
> configure option --enable-vary-options.

Unless there is non-trivial process required for regular vary headers
with this enabled, I don't think it needs to be optional.

-Rob

-- 
GPG key available at: .


signature.asc
Description: This is a digitally signed message part

Re: two xasserts in squid3

[Amos Jeffries]

> Alex Rousskov wrote:
>> On Sun, 2008-02-10 at 20:51 +0200, Tsantilas Christos wrote:
>>> Maybe it was better if the files lib/assert.c and include/assert.h
>>> removed and the assert macro  defined in squid.h file like squid2.6
>>> does...
>>
>> It looks like Array, MemPool, and splay files are using assert outside
>> of src/ so you will not be able to remove include/assert.h. However, you
>> may be able to declare xassert in include/assert.h while defining it in
>> src/Debug.cc.
>>
>
> It is not the best to have the declaration at library headers
> (include/*) and the definition in program sources, but yes looks that it
> is the safest solution at this time.
>
> Should I fix it in async-calls branch, or someone else will do the job
> in squid3-HEAD?

I have yesterday created a branch "cleanup" in Squid-3 for the purpose of
performing the larger cleanup operations instead of HEAD.
This is so we can maintain better stability in HEAD and the bigger or more
experimental changes can be done and tested in cleanup before going
mainstream.

Feel free to do it there and see what breaks.

Amos

>
>> The assert #define in src/Debug.h should probably be moved to
>> include/assert.h. These #defines must be the same.
>
> Are not exactly the same. In the Debug.h implementation there is an
> extra #if PURIFY ".
> The "assert.h" file included at the begining of squid.h file and the
> Debug.h included in defines.h file which also included at the squid.h.
> It is not clear with a first view which assert called, and I wondering
> if there is a trick here..
>
>
>
>

Re: two xasserts in squid3

[Tsantilas Christos]

Alex Rousskov wrote:
> On Sun, 2008-02-10 at 20:51 +0200, Tsantilas Christos wrote:
>> Maybe it was better if the files lib/assert.c and include/assert.h
>> removed and the assert macro  defined in squid.h file like squid2.6
>> does...
> 
> It looks like Array, MemPool, and splay files are using assert outside
> of src/ so you will not be able to remove include/assert.h. However, you
> may be able to declare xassert in include/assert.h while defining it in
> src/Debug.cc.
> 

It is not the best to have the declaration at library headers
(include/*) and the definition in program sources, but yes looks that it
is the safest solution at this time.

Should I fix it in async-calls branch, or someone else will do the job
in squid3-HEAD?

> The assert #define in src/Debug.h should probably be moved to
> include/assert.h. These #defines must be the same.

Are not exactly the same. In the Debug.h implementation there is an
extra #if PURIFY ".
The "assert.h" file included at the begining of squid.h file and the
Debug.h included in defines.h file which also included at the squid.h.
It is not clear with a first view which assert called, and I wondering
if there is a trick here..

Re: two xasserts in squid3

[Alex Rousskov]

On Mon, 2008-02-11 at 12:18 +0900, Adrian Chadd wrote:
> On Mon, Feb 11, 2008, Amos Jeffries wrote:
> > That only because
> > none has shown me a better way to do function pointers than the way squid
> > currently does them.
> 
> I'm pretty sure the async calls stuff has an implementation of this.
> There's other ways - look at asio.sourceforge.net's implementation which
> uses some Boost methods for creating 0, 1 and 2 argument callbacks
> of varying types.

Do Boost methods for creating callbacks differ substantially from Squid
methods? I was under impression they are about the same except Squid
ones are less polished and simpler because they cover fewer special
cases (that Squid is not using yet).

Thank you,

Alex.

Re: two xasserts in squid3

[Alex Rousskov]

On Sun, 2008-02-10 at 20:51 +0200, Tsantilas Christos wrote:
> Maybe it was better if the files lib/assert.c and include/assert.h
> removed and the assert macro  defined in squid.h file like squid2.6
> does...

It looks like Array, MemPool, and splay files are using assert outside
of src/ so you will not be able to remove include/assert.h. However, you
may be able to declare xassert in include/assert.h while defining it in
src/Debug.cc.

The assert #define in src/Debug.h should probably be moved to
include/assert.h. These #defines must be the same.

When we get a better VCS, we should discuss moving include/ and lib/
stuff into src/ with the exception of 3rd party code. This would avoid
problems created by that artificial boundary.

Alex.

Re: two xasserts in squid3

[Alex Rousskov]

On Mon, 2008-02-11 at 09:33 +1100, Robert Collins wrote:
> One of the things I'd most love to see is the modularisation completed
> - complete deletion of protos.h, structs.h, typedefs.h.

Yes, of course. That would be the focus of the cleanup that Amos is
volunteering to do :-)

Alex.

Re: two xasserts in squid3

[Alex Rousskov]

On Mon, 2008-02-11 at 10:15 +1300, Amos Jeffries wrote:

> I think a better approach to this would be:
> 
> 1) do we actually need it anyway?

Yes, we need a common assert-like macro.

> 2) where is it supposed to be defined?

IMO:

include/xassert.h if xassert is used outside of src/

src/xassert.h otherwise, until we get something like src/base/ or
src/misc for general-purpose commonly used things that do not fit
anywhere else.

> I'm willing to commit time to that if we can agree on the 'old' files
> that need removing and the scope of the remining ones.
> I have seen a few things that made me think squid.h and defines.h may
> need to (merge?) stick around under a better definition of content.

I would welcome such cleanup, especially after we switch to a VCS that
does file renaming.

IMO, there needs to be one header file like squid.h that is guaranteed
to be the first one included by all other source files in src/. That
file should only contain things needed by 99.9% of the code base. This
is usually limited to hacks that change some global system or compiler
effects. The rest should have their own purpose-specific header files.

If the source code needs something used by others, it needs to include
something.h.

There is also a question of whether some lib/ stuff needs to be moved to
src/, but that is a separate debate and we need a better VCS for that as
well.

HTH,

Alex.