Re: Directory

[Christian Reiniger <warewolf@mayn.de>]

Bjarke Hammersholt Roune wrote:

>GetDir(), GetFile() and GetEntry() are implemented. Works just like
>SearchDir(), searchFile() and SearchEntry(), except that only the Directory
>for which they are called is searched, and they take "const char* name"

Ok.

>instead of "const URLInfo& URL". There are two overloads of each, one that
>takes a string and a length, and one that take just a string. The one that
>takes just a string is only one line (ie, it gets inlined). What it does is
>to calls strlen(), then call the overload of itself that takes both a string
>and a length.

Good. That's faster than my method.

>The situation is exactly the same for SearchFile(), SearchDir() and
>SearchEntry(), except that what is taken is a "const URLInfo&", and there is
>only one overload of each. All the SearchXxx and GetXxx methods uses
>iteration rather than recursion.

In SearchXXX also iteration as in 
name = FirstPart ();
Dir = GetDir (name);
while (not_done)
{
    name = NextPart ();
    Dir = Dir.GetDir (name);
}

?

>A place where I changed something is with IsEmpty(). IsEmpty() returns true
>only if the Directory objects contains no entries at all. IsPseudoEmpty() in
>Directory works like IsEmpty() in ppf_Directory (Is IsEmptyRecursive()
>better?). I realize this will make some code need to change, but just
>comment out the IsEmpty() and the compiler should pointer out all the places
>in which this is the case.

IsEmpty () is used for determining whether the the dir can be removed
safely. That's only done during the preparations for mounting and umounting.

>I think the new names for IsEmpty() and IsPseudoEmpty() portrays somewhat
>better what exactly is going on. IsPseudoEmpty() clearly tells people that
>even if it returns true, things are only "kind of empty".

But it doesn't matter ;)

>On another note, how often is this method called? It uses recursion, which

Very seldom.

>is a little ineffecient. I could implement it using a stack, and thereby
>remove the recursion, but if its almost never called anyway, its not worth
>the trouble (and the potential for a bad_alloc from the array that needs to
>be allocated). Its just that I forsee that IsPseudoEmpty() can get to be
>called ALOT of times if you happen to call it for the top directory, and you
>have many directories, REALLY alot (well, ok, it would probably pretty
>quickly find a file somewhere and return, but still).

In 99.99% of the cases it would either (1) immediately find a file or (2)
almost immediately find a dir that's attached to a mounted FS (and thus
cannot be safely removed) or (3) recurse into the one or perhaps (very
rare!) two directories that are the only entries in this dir.

So - even in the worst likely case - calling it for the toplevel dir, so
that the recursion depth is 4 to 15 (very rare) - it's still so cheap that
switching to iteration won't get any measurable speed gain.

>I remove the GetFirstEntry() and ContainsDir(). I've implemented an external
>iterator to replace GetFirstEntry(). That's what its used for, rigth,

Right. Used in ppf_OpenDir () and ppf_PakFileWDir::Cleanup ()

>dirs are seperate). ContainsDir() was obsoleted by the combination of
>storing files and dirs apart, and GetDir() (ie, simply call getDir() with
>the name of the Dir you're looking for, and if it returns non-zero, it
>exists in the Directory).

Ok.

>The two overloads of RemoveFile() look and function the same way they did in
>ppf_Directory. The two overloads of Add() that take either a Directory
>pointer or a File pointer has been supplemented by a overload that takes a
>DirEntry().

Good.

>SetParent() has been changed so that its single Directory pointer parameter
>cannot be 0. The functionality previously given by calling SetParent() has
>been moved into MakeTopMostDir(). The method IsTopMostDir() has been added.

MakeTopMostDir () is unneccessary. There is only one topmost dir, and that
is created during initialization of the global object ppf_GlobalData, with
parent=0 in the constructor.
On the other hand - the "parent" item in the constructor could be
obsoleted. SetParent () is called anyway in Directory::Insert ()

>GetParent() remains unchanged and returns 0 for a top-most dir. Calling
>SetParent(0) will work in release builds, but will otherwise result in an
>assertion (ie, no nasty surprises).

Good.

>Should Directory store the actual Directories and Files, or just pointers to
>them? I'm doing pointers rigth now.

pointers.

>I haven't got to serializing yet. I think it would perhaps be a good idea to
>define virtual methods SerializeFrom(FILE*) and SerializeTo(FILE*) in
>Directory... ? Else we have all serializing functionality stuck in Misc
>(which isn't very intuitive), and we can't expand upon it in derivatives of
>Directory.

Agreed. But I don't know if a simple SerializeTo () will work. Hmm, Pak F1
updating isn't done via serialize anyway, so it should be ok.

>Oh yeah, as soon as you get those headers done, I need them. Rigth now I'll
>have to hack the headers somewhat to get them to accept my compiler (those
>"#error you need POSIX" are pretty annoying") and to get it to include
>"ppfWin32PosixImpl.h".

I completely removed the #include <unistd.h> thing from the headers.
Instead a symbol PP_SYS_POSIX is defined (or not...)

>I remember you talking about creating a wrapper that only included the
>features we need, so that we could encapsulate platform-specific directory
>handling code. I think this is *definately* the way to go. For now
>ppfWin32PosixImpl.h will have to do, though.

Right.


	Christian
-- 

Drive A: not responding...Formatting C: instead