Re: PSound updates

["Peter Burns" <pburns@firstdata.com.au>]

>Ok, there are some unclear points with your changes:

>* In OpenDir.cpp you enclose the
>       if (Dir->Entry->NameIsCopy != 0)
>              delete [] Dir->Entry->Name;
>  lines in #if 0 , #endif. Why? Doing so creates a memory leak.
>* Same file, in functions ReadDirVirt and StatVirt you changed
>    Entry.Name       = DirE->GetName ();
>    Entry.NameIsCopy = 0;
>  to
>    Entry.Name       = Strdup(DirE->GetName ());
>    Entry.NameIsCopy = 0;
>  Copying the string isn't neccessary at all here, as the original
>  (DirE->GetName ()) won't disappear while Entry.Name is used.
>  Also, *if* the string is copied, Entry.NameIsCopy has to be set to
>  != 0, so that it can be deleted again later (see the first point)

This problem has to do with constness.
DirE->GetName() returns a const char*
Dir->Entry->Name is a char*
The changes that I made here don't really solve the problem.
This was just something I did in a hurry. I didn't write the original code
and I still don't know it very well.



>* 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. That extra bit of code changes it from /d/some/path to
d:/some/path

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.

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.


>>>       path = copied_path;

>That's wrong. The "path += offset;" line sets "path" to the right point
>*in* copied_path (behind the leading slashes). Theoretically. I'll change
>it to a version that works a bit better...

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;
     }
<<<<<<<

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:/).

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

>241a243,245
>>       // the leading / always gets stripped off
>>       // so a dos type path looks like c/blah/blob.txt
>>
>256a261
>>       part_start[0]=&Path[2];

>Hmmmm......
>No. part_start[1] already points to Path [2]. So you "duplicate" one part
>here. correct would be
>part_start [0] = &Path [0] (with the Path pointer corrected to point to the
>real start of the path).
>But it *is* correct to change part_start[0] here.
>Fixing.


>>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.