[try2] advapi32: Implement CredReadDomainCredentials stub and tests

[James Hawkins]

On Fri, Oct 10, 2008 at 12:59 AM, Andrey Turkin <andrey.turkin at gmail.com> wrote:
> James Hawkins wrote:
>
> On Thu, Oct 9, 2008 at 4:14 PM, Andrey Turkin <andrey.turkin at gmail.com>
> wrote:
>
>
> [now with tests, fixed and reformatted as per James' suggestions]
>
> Implement CredReadDomainCredentialsA and CredReadDomainCredentialsW
> stubs and few tests for them.  Required for MSN Messenger 7.0
> ---
>  dlls/advapi32/advapi32.spec |    4 +-
>  dlls/advapi32/cred.c        |  160
> +++++++++++++++++++++++++++++++++++++++++++
>  dlls/advapi32/tests/cred.c  |   45 ++++++++++++
>  include/wincred.h           |   39 +++++++++++
>  4 files changed, 246 insertions(+), 2 deletions(-)
>
>
>
> +    *Size = 0;
>
> +    *Credentials = NULL;
>
> +    if (!TargetInformation)
>
> What if Size or Credentials is NULL?  You should add a test for that
> and handle it appropriately.  If the tests crash, you should comment
> out the tests and note that there is no handling in Windows.
>
>
>
> Why? Windows does the same thing, tests would crash. Do you really think
> somebody need to know this? Is there any application that depends on this
> behavior so these assignments must be commented to protect them from change?
> I can't see any value in commented out test or sort-of-meaningless code
> comment.
>

The tests serve as documentation of the API.  In many cases, that
documentation is far superior to even msdn.  Just because you know
that the implementation matches native doesn't mean that someone else
looking to work on the function knows that.  He'll then waste time
figuring out what you already know.  Ask yourself the opposite: what's
the harm in adding such documentation?

-- 
James Hawkins