Re: Add TRACEs to NtCreateFile returns (second try)

[Markus Hitter]

Am 26.08.2008 um 01:33 schrieb Juan Lang:
> It seems your tolerance for reworking patches is rather low.

Currently, yes. I'll have to take a breath.
> - if (!attr || !attr->ObjectName) return STATUS_INVALID_PARAMETER;
> - if (!attr || !attr->ObjectName)
> - {
> -
>    TRACE("returning STATUS_INVALID_PARAMETER\n");
> -
>    return STATUS_INVALID_PARAMETER;
> - }
>
> I agree with James that this is superfluous, or at least that it
> doesn't match the style of dlls/ntdll/file.c:  when error is
> encountered, e.g. in NtReadFile line 568, it's returned directly.

It's returned, but won't show up in the console. I thought the idea  
of TRACE()s is to allow some sort of cheap printf()-style debugging  
and should be complete. Perhaps there are other uses I didn't figure  
yet.
>  if (attr->RootDirectory)
>  {
>      FIXME( "RootDirectory %p not supported\n", attr- 
> > RootDirectory );
> -
>    TRACE("returning STATUS_OBJECT_NAME_NOT_FOUND\n");
>
> Adding a trace after a fixme is clearly superfluous.

OK.
> -
>    if (io->u.Status == STATUS_SUCCESS) io->Information =  
>
> FILE_OPENED;
> -
>    if (io->u.Status == STATUS_SUCCESS)
> -
>    {
> -
>        io->Information = FILE_OPENED;
> -
>        TRACE("returning STATUS_SUCCESS, handle %p\n", *handle);
> -
>    }
> -
>    else
> -
>        TRACE("returning 0x%08x\n", io->u.Status);
>    return io->u.Status;
>
> Since the return is done regardless if the status, why do you trace a
> different value depending on the status?

Because *handle is valid on success only and to not confuse the  
observer with invalid handles. Perhaps this is more care than needed.
> Please try to minimize the changes you're making

Sure. After the breath :-)

MarKus

- - - - - - - - - - - - - - - - - - -
Dipl. Ing. Markus Hitter
http://www.jump-ing.de/