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