Add TRACEs to NtCreateFile returns (second try)

Juan Lang juan.lang at gmail.com
Mon Aug 25 18:33:35 CDT 2008


> So, why not fix this? Please push the patch trough Wine's patch accepting
> mechanism yourself, I'm currently somewhat sick of it.

It seems your tolerance for reworking patches is rather low.  You must
be willing to rework patches until they're correct to have many
committed around here.  I've probably had more patches rejected than
you've sent so far, so I know whereof I speak ;-)

>> My comment still stands that the added TRACE is absolutely superfluous.
>
> Makes 2 pro, 1 neutral. Do whatever you want with it.

James is the wrong target for this comment, and voting is irrelevant.
The burden is always on you to convince AJ that your patch is correct.

Since you're new-ish around here, I had a look.

-    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.

     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.

-        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?  One trace just before the
return should suffice.  Same with the remaining returns:  a single
trace just before the return should suffice.

Please try to minimize the changes you're making, pay attention to the
style of the file you're changing, and be receptive to comments you
get from other developers.
--Juan



More information about the wine-devel mailing list