Re: current state of bzr repository

Amos Jeffries wrote:
>
>> - src/Debug.h The HERE macro is wrong. The original one was not bad....
>
> Original one sometimes left strings with confidential build info in other
> peoples cache.log....
>
> We spent a long while covering the possibilities to get the current one.
> If you'd like to trace down what cases are broken we can try for a better
> fix.
>
OK. I am getting the follow:
    assertion failed: debug.cc:763: "strstr(file,ThisFileNameTail)!=NULL"
The file points to "debug.cc" and the ThisFileNameTail points to
"src/debug.cc"

But is not the only problem I am seeing.
In the case of the gcc the __FILE__ macro is the source file passed as
argument. For example:
   if you use "g++ -c debug.cc" to compile your source file the
__FILE__="debug.cc".
   if you use "g++ -c /path/to/debug.cc" then the
__FILE__="/path/to/debug.cc".

In our case now, from what I can understand the problem you are
referring appeared on compilers other then gcc or when autotools for a
reason use the full path name to compile the sources eg:
   g++ -c `pwd`/debug.cc

Now imagine the case some files compiled using the
    g++ -c `pwd`/afile.cc
and some others using the
    g++ -c afile.cc

In the first case the __FILE__="/path/to/af.cc" and in the second
__FILE__="af.cc". In the second case in the SkipBuildPrefix function
the line:
     return path+BuildPrefixLength;
is problematic (maybe the BuildPrefixLength is 9 for example).

I believe the SkipBuildPrefix is not safe as is. Maybe the best is to
use strrchr to find the last "/", if exists, and compute the basename
using the result.

>> - The ufs aufs diskd does not link well. An src file like the
>> src/FsReg.cc in my patch should added and fix a litle the libaufs.a
>> libufs.a and other libraries (probably should included in one library)
>
> This is not possible at present. So long as admin have the option of
> specifying custom lists of FS (--enable-storeio) some of the FS may not
> exist at build time, and some custom ones we don't register in FsReg.cc
> may be added.
>
> The HAVE_* defines like those used for auth modules will have to be done
> for FsReg.cc first.

Currently the libaufs.a library is only one code line:
   static StoreFSufs<UFSSwapDir> AufsInstance("DiskThreads", "aufs");

Maybe the best is to remove the libaufs and libdiskd libraries and move
the declarations of AufsInstances DiskdInstance and UfsInstances in a
FsReg.cc file. Always using the required "HAVE_*" defines....

>
> Also the DiskIO modules will need to be updated the same way and the
> DiskIO/DiskIOModules_gen.cc / pokeAllModules() hack will have to be
> removed at the same time.
>
>
> - The ntlm,basic digest schemes does not included in squid binary. A
> file like the src/AuthReg.cc in my patch should added
>
> Auth has the same problem as FS, but the HAVE_* hack kinkie added a while
> ago make your AuthReg an option now.
>
>
> Hm, I think we should carefully re-consider the use of globals now that we
> are explicitly forcing registration.
>
> For now okay. But the use of globals for reg needs a review

I agree.
What about the AuthReg.cc file? Should we wait other developers opinion?

Regards,
     Christos
Received on Wed Mar 25 2009 - 13:54:34 MDT