Re: PSound updates

[Christian Reiniger <warewolf@mayn.de>]

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)

* In Win32.cpp: ChdirPlain () always gets a native path as input. That's
guaranteed. So no need to convert the string in there.
Also, you call free () on a string created via Strdup (), which uses new[].
That's dangerous.

>Half of may be just changes to whitespace.
>The others are to do with dos style paths. The comments don't match up
>to what the code is doing eg URLInfo::ToNative.

<looking some more> Hmmm, you're correct. The problem is in the
		Path [0] = Path [1];
		Path [1] = ':';
It will convert "/c/some/dir" to "//:some\dir", because Path points to the
char *after* all leading slashes. Fixing.

>There are also problems with some copied path not getting a zero at the
>right place in the end.

In Freeze () I guess. Right. That code should use Strdup. Fixing.

>175c174
>< 
>---
>>       memset(copied_path, 0, copied_path_size);

That's in Freeze (). Obsoleted by using Strdup (which is also faster)

>179c178,180
>< 
>---
>>       // put a zero on the end to make a null-terminated string
>>       copied_path[copied_path_size - 16 + 1 + 1] = 0;

unneeded, as either the memset () you chose or the Strdup () take care of
that.

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

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

>You can use this perl script to fix up the line endings:
>
>#!/usr/local/bin/perl -pi
>s/\r//g

Thanks.

>I'm debugging buggy code that I haven't written. I don't have to do
>this. I've got better things to do with my time. 

(Note: I didn't read this far until now, so please excuse that I didn't
address this point earlier)

Sorry if my mails in the past weeks sounded rude. They weren't intended to.
Really. I'm *very* grateful that you've done all this.

You see, when this thing started, I knew that the code wouldn't compile on
VC5, but I had a note about a compile on VC6 from you. So you were the only
one of us who could prepare a binary Win32 package of LibPPlay. I thought
that would be relatively simple, given that you already got things to
compile etc. I guess I was wrong.

Sorry again. It was a misunderstanding. Please don't take it too serious.

And thanks for the debugging you've done.

>I only had 3 hours sleep on friday night (I went to a clan analogue
>thing).

I hope it was fun.


	Christian
-- 

If you can't make it good, make it LOOK good.-Gates.