[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Miscellaneous stuff

Bjarke Hammersholt Roune wrote:

>-ppfWin32PosixImpl problems
>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 dossification
>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.

Same method:
	/* 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.

In ToVirt:
	/* 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
>the behavior).

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'll do.

>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
*and* PenguinSound.

>I also included the fixes I mention in the other E-mail.



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