Re: PSound updates

[Christian Reiniger <warewolf@mayn.de>]

Peter Burns wrote:

>This problem has to do with constness.
>DirE->GetName() returns a const char*
>Dir->Entry->Name is a char*

Not in my copy (the one that's also in CVS). 
Dir is of type ppfDIR (defined in PenguinFile.h)
Dir->Entry is of type ppfDirEntry (also defined in PenguinFile.h)
Dir->Entry->Name is of type const char * here.
Perhaps you changed that before to work around different constness problems?

>>* In Win32.cpp: ChdirPlain () always gets a native path as input. That's
>>guaranteed. So no need to convert the string in there.
>
>Going through with the debugger reveal that it receives a string like
>/d/some/path. 

Then that's a bug in the calling code. <looking>
This is the only code calling ChdirPlain (in FileGlobalData::SetCWD ()):
---
	URLInfo NativePath;

	try {
		NativePath.Init (Path);
		NativePath.ToNative ();
	} catch (std::bad_alloc) {
		return -2;
	}

	if (ChdirPlain (NativePath.GetPath ()) == 0)
---

I.e. the PFile-style path "Path" is converted to a native one before the
call to ChdirPlain. Likely some bug in URLInfo::ToNative ().
Did you test this with the changes to URLInfo I committed last night?
They include some changes (primarily to URLInfo::ToNative ()) that were not
in your diff.

>Most of the problems with the code have to do with the drive letter.
>
>d:/some/path gets modified to /d/some/path. The initial / gets chopped off
>and the path becomes d/some/path. The path is then given as a *string* to
>URLInfo::Init which assumes that it is a relative path.

That's bad. Where does this happen? I just checked a bit around the typical
Chdir callchain and didn't see anything like this.

>I managed to get pfile_basic to work - it displays success - but there are still
>some warning messages.
>URLInfo::ToNative complains that d:/some/path can't be converted to native.
>I guess I haven't quite done things properly when putting the drive letter back
>in
>the right place.

>I think that the code could be improved by breaking some of the functions
>down into smaller ones.

Agreed. There are some rather big and ugly ones. Do you have specific ones
in mind?

>When I did this I didn't under stand what this little bit of code was doing:
>>>>>>>>
>ppOffsetT offset = copied_path - original_path;
>path += offset;
>
>int i;
>for (i=0 ; i < part_count ; i++)
>     {
>     part_start [i] += offset;
>     }
><<<<<<<

Um, yes, well, the idea was that, well, oh, forget it. It was sick. The
code now in CVS contains a clearer (and safer) version.

>All I knew was that it wasn't working properly, due to changes to the path
>that weren't refelected in part_start (ie /d/ to d:/).

Oops. Good that you mentioned it again. There's still a bug in there. I'll
commit a fix when sending this mail.

>The changes I make often don't modify other things that they are supposed to,
>as I don't know enough about the code. eg this little thing about part_start and
>part_length

part_start and part_length are simply some info about the individual
"sections" of an URL. Example:

/    <- original_path (or copied_path) points to this
s    <- path and part_start [0] point to this point
o
m
e       part_size [0] == 4 (strlen ("some"))
/
/    uninteresting additional slash - ignored
w    <- part_start [1]
e
i
r
d       part_size [1] == 5 (strlen ("weird"))
/
.
.
.

I.e. some data faciliating "traversing" the path. Some functions e.g. take
an URLInfo& as parameter plus an int indicating which part part they should
process (Directory::SearchFile (URL, i) looks in its directory for the i-th
part of the URL. If that's a directory, it calls SearchFile (URL, i+1) on
that dir. Makes the thing relatively fast and clean)

>>>I only had 3 hours sleep on friday night (I went to a clan analogue
>>>thing).
>
>>I hope it was fun.
>
>It was. I'll definitely try and go to the next one. Music sounds
>so much better when you can feel it.
>I liked Sub Bass Snarl the best.
>B(if)tek, and 5000 Fingers of Doctor T were good as well.

Ummm, yes. Good.
To be true I don't have the faintest idea what you're talking about ;+)
When reading "clan" I thought of either some network session or a family
meeting...


	Christian
-- 

Windows IS NOT a virus...viruses do something.