Add TRACEs to NtCreateFile returns (second try)

Markus Hitter mah at jump-ing.de
Tue Aug 26 05:06:58 CDT 2008


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/







More information about the wine-devel mailing list