Re: Reparse point unit tests patches

2009-04-27 Thread Paul TBBle Hampson
On Sun, Apr 26, 2009 at 12:39:31PM +0200, Detlef Riekenberg wrote:
 On So, 2009-04-26 at 13:18 +1000, Paul TBBle Hampson wrote:

 [PATCH 2/2] Add unit tests for junction points using reparse point interface
 http://www.winehq.org/pipermail/wine-patches/2009-January/067227.html

 I didn't test the patches, but from the quick lock:

 The test must compile with the Microsoft VC toolchain.
 That might break, when you define a struct in your source without
 a guard #ifndef.
 Much better is the use of the correct header for that.

+ /* Stuff that lives in ntifs.h, according to MSDN */
 It's in ddk/ntifs.h in OpenWatcom.

Sounds good, I'll stick it there in Wine. I had a similar suggestion
before as I recall, but didn't get any confirmation of that idea at the
time.

The problem however is that as I recall, ntifs.h isn't distributed
freely (or maybe it is now?) so most Visual Studio users won't have a
copy of it by default. The usual suggestion I've seen on the 'net for
this sort of thing is pretty much what I did, inlining the definitions
that are normally in ntifs.h.

But I prefer this way 'round, in the end.

I had to create ntifs.h, so I've guessed the guards as _NTIFS_ (based on
ddk/ntddk.h), if anyone wants to tell me what the right guard is, or
that I can actually go and pull that information out of the actual DDK
headers, that'd be great.

I'll merge that header into the first patch rather than the second as
well, methinks.

 + buf[bytes] = '\0';
 Your debugstrn_w is broken.
 Your Code writes to unallocated memory, when WideCharToMultiByte need
 more bytes as you provided.

Huh, yeah. I probably should remove that whole #if 0 block, except it's
rather useful for diagnosing errors. I think I'll rip it out for now,
save it until it's both useful and better written.

+ (WCHAR*)(((char*)
 That looks really ugly.

It is. The problem is the PathBuffer is a WCHAR array, but the offsets
are in bytes. It's awful, true.

Anyway, that's also going away for now. If I recreate such code, I'll
see about doing it prettier.

+ static void InitFunctionPointers(void)
 A private function with mixed case can be mixed up with an Windows API
 function,
 when used. Please use lowcase here.

Fixed.

 Please avoid to use the current Directory.
 I suggest to use a temporary directory below GetTempPath and create your
 test directories there

I'll look into doing that, but it might take me a little while to get
put together and retested.

+ skip(kernel32 does not export required functions.\n);
 Please use win_skip here

Oh, OK. Yeah, I see how that works. That way if we skip under Wine,
it'll still be a failure, but if we skip under Windows it'll be a skip
(since it'll only happen on downlevel Windows boxes).

Thanks for all your comments, I've applied all but one to my local tree
already, I'll see if I can put together the GetTempPath changes this
week. (I have to go learn how to use GetTempPath first. ^_^)

-- 
---
Paul TBBle Hampson, B.Sc, LPI, MCSE
Very-later-year Asian Studies student, ANU
The Boss, Bubblesworth Pty Ltd (ABN: 51 095 284 361)
paul.hamp...@pobox.com

Of course Pacman didn't influence us as kids. If it did,
we'd be running around in darkened rooms, popping pills and
listening to repetitive music.
 -- Kristian Wilson, Nintendo, Inc, 1989

License: http://creativecommons.org/licenses/by/2.5/au/
---


pgpIIkCNoMhlq.pgp
Description: PGP signature



Re: Reparse point unit tests patches

2009-04-26 Thread Detlef Riekenberg
On So, 2009-04-26 at 13:18 +1000, Paul TBBle Hampson wrote:

 [PATCH 2/2] Add unit tests for junction points using reparse point interface
 http://www.winehq.org/pipermail/wine-patches/2009-January/067227.html

I didn't test the patches, but from the quick lock:
 
The test must compile with the Microsoft VC toolchain.
That might break, when you define a struct in your source without
a guard #ifndef.
Much better is the use of the correct header for that.

+ /* Stuff that lives in ntifs.h, according to MSDN */
It's in ddk/ntifs.h in OpenWatcom.

+ buf[bytes] = '\0';
Your debugstrn_w is broken.
Your Code writes to unallocated memory, when WideCharToMultiByte need
more bytes as you provided.

+ (WCHAR*)(((char*)
That looks really ugly.

+ static void InitFunctionPointers(void)
A private function with mixed case can be mixed up with an Windows API
function,
when used. Please use lowcase here.

Please avoid to use the current Directory.
I suggest to use a temporary directory below GetTempPath and create your
test directories there

+ skip(kernel32 does not export required functions.\n);
Please use win_skip here


-- 
 
By by ... Detlef





Reparse point unit tests patches

2009-04-25 Thread Paul TBBle Hampson
I posted a series of patches to add reparse point headers and
junction point unit tests, as a basis from which to fix bug 12401.

Is there any chance of these going in as they are, or do I need to sit
on them until I or someone else implements the functionality they're
testing?

I'm linking to the old version, but git has been happily rebasing them
for me up to and including 1.1.20, so they should still be applicable,
or I can repost the latest versions.

I suspect the relevance of this patch has changed since the change of
fixed disk reported filesystem from NTFS to UnixFS. (ie bug 10601 isn't
dependant on bug 12401 after that change, see bug 10601 comment 23) but
I think it's a good thing to have in the Wine tree anyway.

[PATCH 1/2] Add definitions used by reparse points
http://www.winehq.org/pipermail/wine-patches/2009-January/067226.html
[PATCH 2/2] Add unit tests for junction points using reparse point interface
http://www.winehq.org/pipermail/wine-patches/2009-January/067227.html

-- 
---
Paul TBBle Hampson, B.Sc, LPI, MCSE
Very-later-year Asian Studies student, ANU
The Boss, Bubblesworth Pty Ltd (ABN: 51 095 284 361)
paul.hamp...@pobox.com

Of course Pacman didn't influence us as kids. If it did,
we'd be running around in darkened rooms, popping pills and
listening to repetitive music.
 -- Kristian Wilson, Nintendo, Inc, 1989

License: http://creativecommons.org/licenses/by/2.5/au/
---


pgpW9GwPM5Zq5.pgp
Description: PGP signature