Reparse point unit tests patches

Paul TBBle Hampson Paul.Hampson at Pobox.com
Mon Apr 27 02:01:23 CDT 2009


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.Hampson at 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/
-----------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20090427/abccb803/attachment.pgp>


More information about the wine-devel mailing list