Re: crypt32: CryptProtectData/CryptUnprotectData take 2

[Mike McCormack]

Kees Cook wrote:
> ChangeLog:
>     Black-box implementation of CryptProtectData/CryptUnprotectData.
>
> Here is an updated patch with various recommendations implemented.

Hi Kees,

The new patch looks good.  I should have mentioned before that writing a 
test case will help your patch be accepted.  Did you have any test code 
about that you could turn into a test case for your newly implemented 
functions?

thanks,

Mike

crypt32: CryptProtectData/CryptUnprotectData take 2

[Kees Cook]

On Tue, Apr 05, 2005 at 02:32:11PM +0900, Mike McCormack wrote:
> The new patch looks good.  I should have mentioned before that writing a 
> test case will help your patch be accepted.  Did you have any test code 
> about that you could turn into a test case for your newly implemented 
> functions?

Sure, I can write something.  I'll look around for docs on how to run 
tests -- I didn't find that when I looked around this morning.

Also: I realize I should provide full documentation for the actual 
Windows API calls themselves.  I documented everything BUT those.  :)  
What's the convention for the number after the API name?  I've seen some 
with numbers, and some with just an "@" sign?

crypt32: CryptProtectData/CryptUnprotectData take 2

[Joris Huizer]

Kees Cook wrote:
> On Tue, Apr 05, 2005 at 02:32:11PM +0900, Mike McCormack wrote:
> > The new patch looks good.  I should have mentioned before that writing a 
> > test case will help your patch be accepted.  Did you have any test code 
> > about that you could turn into a test case for your newly implemented 
> > functions?
>
> Sure, I can write something.  I'll look around for docs on how to run 
> tests -- I didn't find that when I looked around this morning.
>
> Also: I realize I should provide full documentation for the actual 
> Windows API calls themselves.  I documented everything BUT those.  :)  
> What's the convention for the number after the API name?  I've seen some 
> with numbers, and some with just an "@" sign?

I think it's an "@" for normal calls, and a number for an 'ordinal' 
call; (though don't really know what the difference is... ;))

HTH,

Joris

crypt32: CryptProtectData/CryptUnprotectData take 2

[Mike McCormack]

Kees Cook wrote:
> Sure, I can write something.  I'll look around for docs on how to run 
> tests -- I didn't find that when I looked around this morning.

The best way to write a test is to look at some of the test cases that 
are there already.  Write and run the test under Windows, and make sure 
it passes on Windows first.  The test is something like this:

/* filename is dlls/crypt32/tests/protect.c */
#include "wine/test.h"

START_TEST(protect)
{
     /* basic form of the test */
     ok(TRUE, "This should be true\n");

/* try with incorrect parameters */
     ok(!CryptProtectData(NULL,NULL,NULL,NULL,NULL,0,NULL), "crypt 
protect data should fail\n");
     ok(GetLastError() == ERROR_INVALID_PARAMETER, "error returned 
incorrect\n");
     ...

/* try with correct parameters */
     ...

/* for tests that pass on windows, but not on Wine */
     wine_todo {
         ok( failing_test(), "this test should pass\n");
     }
}
> Also: I realize I should provide full documentation for the actual 
> Windows API calls themselves.  I documented everything BUT those.  :)  
> What's the convention for the number after the API name?  I've seen some 
> with numbers, and some with just an "@" sign?

Describing the function is good, but IMO, test cases are a more accurate 
description :)  You're probably safe using [crypt32.@], as crypt32 
probably doesn't export functions via ordinal (number).  You can check 
the crypt32.spec file to see.

Mike

crypt32: CryptProtectData/CryptUnprotectData take 2

[Kees Cook]

On Wed, Apr 06, 2005 at 10:29:37AM +0900, Mike McCormack wrote:
> The best way to write a test is to look at some of the test cases that 
> are there already.  Write and run the test under Windows, and make sure 
> it passes on Windows first.  The test is something like this:

Ah!  Whoops, I didn't think to try that originally.  I was just about to 
send my new patch too.  [time passes...]
> /* try with incorrect parameters */
> ok(!CryptProtectData(NULL,NULL,NULL,NULL,NULL,0,NULL), "crypt 
>
> protect data should fail\n");
>     ok(GetLastError() == ERROR_INVALID_PARAMETER, "error returned 
> incorrect\n");

Ah-ha, I need to make calls to SetLastError then.  I wasn't doing that 
either.  Okay, 3rd time's the charm.  This patch includes full 
documentation, as well as the test suite which passes both on Wine and 
Windows.

crypt32: CryptProtectData/CryptUnprotectData take 2

[Jakob Eriksson]

Kees Cook wrote:
> On Tue, Apr 05, 2005 at 02:32:11PM +0900, Mike McCormack wrote:
> > The new patch looks good.  I should have mentioned before that writing a 
> > test case will help your patch be accepted.  Did you have any test code 
> > about that you could turn into a test case for your newly implemented 
> > functions?
>
> Sure, I can write something.  I'll look around for docs on how to run 
> tests -- I didn't find that when I looked around this morning.
>
> Also: I realize I should provide full documentation for the actual 
> Windows API calls themselves.  I documented everything BUT those.  :)  
> What's the convention for the number after the API name?  I've seen some 
> with numbers, and some with just an "@" sign?

Copied from a post yesterday:

Thomas Kho mentioned that
http://www.geekymedia.com/twiki/bin/view.cgi/WineDev/AddingMakefile
was helpful to him.  Since the webmaster there says he's
taking down that wiki soon, here's a copy for posterity.

-- snip --
Topic: AddingMakefile (as part of a new Wine test)

Follow the example of the lzexpand test patch:
http://www.winehq.com/hypermail/wine-patches/2004/11/0182.html

Basic steps. (you'll have to chmod mod most of the files to 644 before 
you can edit them)
   1. In your tests directory, look over wine/dlls/lzexpand/Makefile.in 
and wine/dlls/lzexpand/tests/Makefile.in
   2. Now look at the Makefile.in in your dll directory. Copy it into 
your tests dir and change the list of .c files to your test .c file
   3. Edit the Makefile.in file in your dll directory to add a subdirs 
line (see lzexpand/Makefile.in for an example)
   4. In your wine directory, chmod 644 configure.ac
   5. Edit configure.ac, find your dll directory in the AC_CONFIG_FILES 
and then add the path to your tests directory after it.
   6. In your wine directory, run autoconf. Make sure you have 2.53 
installed (autoconf --version).
   7. Run configure. Your Makefile should be listed at the end now.
-- snip --