[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: PSound updates
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;
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.
>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  = Path ;
Path  = ':';
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.
>> memset(copied_path, 0, copied_path_size);
That's in Freeze (). Obsoleted by using Strdup (which is also faster)
>> // 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
>> 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...
>> // the leading / always gets stripped off
>> // so a dos type path looks like c/blah/blob.txt
No. part_start already points to Path . So you "duplicate" one part
here. correct would be
part_start  = &Path  (with the Path pointer corrected to point to the
real start of the path).
But it *is* correct to change part_start here.
>You can use this perl script to fix up the line endings:
>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
I hope it was fun.
If you can't make it good, make it LOOK good.-Gates.