[freetds] [PATCH] useless CHECK_NULP in dblib.c:tdsdbopen()
James K. Lowden
jklowden at freetds.org
Fri Mar 20 20:27:04 EDT 2009
Craig A. Berry wrote:
> The CHECK_NULP statement removed by the attached patch is not actually
> checking anything except whether random data from the stack is non-
> zero.
One of us is confused, Craig. How about the attached instead?
The macro:
#define CHECK_NULP(x, func, param_num, ret) if (!(x)) { dbperror(dbproc,
#SYBENULP, 0, func, (int) param_num); return ret; }
The function call:
tdsdbopen(LOGINREC * login, const char *server, int msdblib)
The check:
- CHECK_NULP(server, "dbopen", 2, NULL);
expands to:
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. When an application calls dbopen(),
#define dbopen(x,y) tdsdbopen((x),(y), 0)
it's supposed to pass a servername as the second argument. If it doesn't,
there's no chance -- well, very little chance -- that FreeTDS will connect
to the server.
But, you made me look. Sybase says of dbopen():
>>> If server is NULL dbopen looks up the interfaces entry that
corresponds to the value of the DSQUERY environment variable or logical
name. If DSQUERY has not been explicitly set, it has a value of "SYBASE".
<<< [1]
Oh. That must be why getenv("DSQUERY") and "SBYASE" are in
mem.c::tds_alloc_login() and config.c::tds_read_interfaces().
I think it might be better to lift it out to the client library, where the
behavior is defined, and let libtds just do as it's told. What would you
say?
Regards,
--jkl
[1] Gotta love that "or logical name".
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbopen.patch
Type: application/octet-stream
Size: 1500 bytes
Desc: not available
Url : http://lists.ibiblio.org/pipermail/freetds/attachments/20090320/85abe3e6/attachment.obj
More information about the FreeTDS
mailing list