[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Miscellaneous stuff
Bjarke Hammersholt Roune wrote:
>First off, there is both a ppfWin32PosixImpl.c and a ppfWin32PosixImpl.cpp
>in the tarball. There should only be a .cpp file. To make matters worse,
>these are both versions of ppfWin32PosixImpl that don't work nor compiles.
Actually there should be *none*. All that stuff should be in Win32.cpp.
>I *really* think we should removed the virtual methods GetSize(),
>AsDirectory() and AsFile() from DirEntry.
>GetSize() makes absolutely no sense what-so-ever for Directory, and it
>therefore does not belong in a base class that File shares with Directory.
Well, In Un*x, directories *do* have sizes, partially because directories
*are* a special kind of file there. Having GetSize () simply return 0 for
Dirs is fine IMHO, and it makes querying the basic Dir Entry attributes
(and the size is a basic thing) easier.
>AsDirectory() and AsFile() makes the virtual table larger than nessecary,
>and are ineffecient anyway. There really is no point in returning a value
I'll remove them. They are leftovers from the days when DirEntry was a base
class for ppf_DirTreeNode instead of Directory and File. They're useless
>URLInfo works perfectly for both dos and unix style paths. You might want to
>give it a quick test that things work the way you intend for them to do,
In ToPlain you assume that the path is always at least 2 chars long, which
isn't guaranteed ("/" is a valid absolute path). Fixing that.
/* Right now the parsing is still valid, but later versions have to
* set is_parsed to false here */
// FIX: why that? (Bjarke)
is_parsed marks whether the part_count, part_start etc fields are valid.
But native paths aren't neccessarily parsed (because they're only given to
stdio anyway, which doesn't use that info). I'll take care of that.
/* Error or undefined behavior in this case
if (fs == ppfFS_plain)
ppInternalError ("ToVirt () called for plain URL \"%s\"",
// FIX: Why that? It works fine for dos-style paths.
Good question. IIRC the original thought was that converting to Virt
doesn't make sense for plain paths, but that's wrong. I'll remove that code.
Also in ToVirt: you don't skip the leading '/' of the Virt'ed path. Fixing
ToAbsolute () needs some fixes, together with the CWD storing in
FileGlobalData. My job.
>Why doesn't ToVirt() work for native paths? I made it work for dos-style
Its sole purpose is to convert native into PFile paths if neccessary.
What exactly do you mean?
>ones. Unix-style ones seem to work ok, except the leading / is trimmed away
>for absolute paths (this very much seemed intentional, so I didn't change
It is intentional. The leading '/' isn't needed because is_absolute clearly
identifies the path as absolute anyway. Keeping the leading '/' would only
make processing code more complex/unintuitive.
>I have changed is_for_plain to is_native and supplemented IsForPlain() and
>ToPlain() with IsNative() and ToNative(). I havne't removed IsForPlain() and
>ToPlain() as it would break code all over.
>I have done the dossification with #ifdef's. If you have a good reason for
>implementing this stuff in seperate methods (ie, why does it matter to a
No, I guess it's fine now. I expected the changes to be more complex.
>The source is coming in priv. mail. I included the project file as you
>requested. If you want it to have another name, or its location is wrong,
>please tell me, as that matters (well, you CAN rename it, but the project
Somehow, yes. You and Peter need to get together to make the project files
cover the *entire* libpenguinplay, i.e. PenguinFile *and* the generics
>I also included the fixes I mention in the other E-mail.
Drive A: not responding...Formatting C: instead