[freetds] [PATCH] useless CHECK_NULP in dblib.c:tdsdbopen()

Frediano Ziglio freddy77 at gmail.com
Mon Mar 23 07:09:10 EDT 2009


2009/3/21 James K. Lowden <jklowden at freetds.org>:
> Craig A. Berry wrote:
>> >>       if (!(server)) { dbperror(dbproc, 20176, 0, "dbopen", (int) 2);
>> >> return (void *)0; };
>> >>
>> >> The check isn't against "random data from the stack"; it's against
>> >> the
>> >> server argument.
>> >
>> > I really wasn't clear (and hadn't looked at the macro long enough to
>> > understand it).  It's dbproc, not server, that has been "fetched but
>> > not initialized".  I'll try to actually think before replying further.
>>
>> Your patch looks good to me, compiles fine here, and I'm now testing
>> with it.  You were right that I was confused in saying that the
>> uninitialized data is what we were checking (server), when in fact
>> it's what we were using to report the error (dbproc).  This was rather
>> dangerous because dbperror dereferences dbproc and could either get
>> garbage or segfault when given a bad pointer.  Had we initialized
>> dbproc to NULL, that would've been ok as dbperror handles that case.
>
> Ah!  I didn't even notice that.  Friggin macros!
>
> Keep 'em coming, Craig.
>

I think that a

DBPROCESS *dbproc = NULL;

is better... so we check parameter and in case server is NULL we call
dbperror with proper DBPROCESS (which is NULL).
I don't understand why gcc don't detect this...

freddy77


More information about the FreeTDS mailing list