On Mon, Aug 25, 2008 at 3:12 AM, Markus Hitter <[EMAIL PROTECTED]> wrote: > > Am 25.08.2008 um 01:31 schrieb James Hawkins: > >> 2008/8/24 Markus Hitter <[EMAIL PROTECTED]>: >>> >> >> + if (!attr || !attr->ObjectName) >> + { >> + TRACE("returning STATUS_INVALID_PARAMETER\n"); >> + return STATUS_INVALID_PARAMETER; >> + } >> >> >> These are all very useless TRACES, except for possibly the returned >> handle value. > > Well, the idea is to TRACE() something for all possible return values. > Michael Karcher, Rob Shearman and me obviously consider them as useful: > > <http://thread.gmane.org/gmane.comp.emulators.wine.patches/54527/> >
I doubt Rob was agreeing with the above trace. He merely clarified how you should print variable values and returned handles. >> As a side note of something I just noticed, the check for NULL attr will >> never be true because we'll crash in the TRACE when we dereference attr. > > In case of (attr == NULL), same as (!attr), the code right to the || > shouldn't be reached, so no dereferencing should take place, then. > TRACE()/printf() is capable of handling 0/NULL/nil values as well. > No, take a look at the TRACE again. If attr is NULL, you'll crash in the TRACE. Neither TRACE nor printf handle NULL strings; debugstr_a/w is what handles NULL strings, but that's beside the point I'm making, as we're talking about attr being NULL, not attr->ObjectName. > BTW., the TRACE() in the code you cited doesn't dereference anything and for > the other parts of the patch, dereferencing only takes place where the > current code dereferences anyways. > Thus why I said "as a side note." It had nothing to do with your patch. -- James Hawkins