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

Frediano Ziglio <[email protected]>
Newsgroups gmane.comp.db.tds.freetds
Message-ID <[email protected]>
2009/3/21 James K. Lowden <[email protected]>:
> 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
_______________________________________________
FreeTDS mailing list
[email protected]
http://lists.ibiblio.org/mailman/listinfo/freetds
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.