FreeTDS Development Group

[Frediano Ziglio <freddy77 AT gmail.com>]

2010/1/9 James K. Lowden <jklowden AT freetds.org>:
> James K. Lowden wrote:
>> It's not
>> a complete answer because (among other reasons) the db-lib error handler
>> won't get the correct error string.
>
> Hi Freddy,
>
> A question for you on this topic.
>
> tdserror() assumes errnum can be converted to a string with strerror(3).
> On Windows that's not true; for winsock errors strerror(3) returns
> "Unknown error".  Somehow, we need to pass the Win32 error string
> (returned from tds_prwsaerror()) to the client libraries.  But how?
>

The problem is that in Windows C errors (from errno) and system errors
(from GetLastError/WSAGetLastError or returned from APIs depending on
API set!) are not compatible! Errors we detect are from errno and
sockets. However there is no way to distinguish. In dblib structure we
have a simply oserr (and also in TDSSOCKET but currently is used only
for socket error that is systems errors). strerror works for C errors
but not for sockets. System errors messages can be read using
FormatMessage API (not that straight but I have example codes).
Currently I added a macro for sock_strerror so it's easier to
distinguish between strerror and sock_strerror.

> The db-lib call stack is:
>
> connect(2) (e.g.)
> tdserror(TDSCONTEXT, TDSSOCKET, msgno, oserr)
> _dblib_handle_err_message(TDSCONTEXT, TDSSOCKET, TDSMESSAGE)
> dbperror (DBPROCESS, msgno, oserr)
>
> I don't want to change the interface to dbperror() or tdserror() just for
> Win32.
>
> My suggestion: add
>
>        char * winsock_errstr
>

I don't like this... perhaps would be better to understand if is a
socket error from tds error code.

> to TDSSOCKET.  Normally it's NULL.  If libtds detects a win32 socket
> error, it looks up the const static string with tds_prwsaerror() and
> stores the pointer in winsock_errstr.  When dbperror() calls the client
> handler, it examines that pointer.  If not NULL, it passes it to the
> client handler instead of calling strerror(3).  To avoid confusion,
> dbperror() sets the pointer to NULL after the handler returns.
>
> I think this would work with the other client libraries, too.  What do you
> think?
>

Looking at windows constants I would use a trick and an unique error
number. C errors in windows are very few (the higher constant is 80
but is not used by FreeTDS) while socket errors are usually higher
than 10000. Perhaps we could define a tds_strerror that take this
constants and output a string. (see
http://msdn.microsoft.com/en-us/library/t3ayayh1%28VS.80%29.aspx and
http://msdn.microsoft.com/en-us/library/ms740668%28VS.85%29.aspx).

Another problem is thread safety. strerror is not thread safe, it
would be better to use strerror_r if available. Under Windows for C
errors there is _sys_nerrs and _sys_errlist.

> BTW, I'm working on updates to the UG for 0.92.
>

Good! I saw you committed patch for defncopy and Windows, it was one
of patches I had on my list (you sent me defncopy.c many time ago but
got no time to check it). I saw some strange include in defncopy like
replacements.win32.hacked.h, win32.microsoft directory. Also there is
a strange types.h inclusion in src/tds/token.c... quite strange,
tds_get_varint_size should be defined in data.c (which include
types.h), this breaks my cross Windows compile. Also I got a double
definition cause you added a strncasecmp in include/replacements.h.
Perhaps should be placed in defncopy.c (like strcasecmp define?).

Did you see my initial patch for forbid instance and port
specification on freetds.conf?? I attach it.

> Regards,
>
> --jkl
>

freddy77
Index: freetds83/include/tds.h
===================================================================
--- freetds83.orig/include/tds.h	2010-01-10 00:42:03.991647287 +0100
+++ freetds83/include/tds.h	2010-01-10 00:42:04.821464901 +0100
@@ -287,7 +287,8 @@
 		TDSEICONVAVAIL = 2401, 
 		TDSEICONVO     = 2402, 
 		TDSEICONVI     = 2403, 
-		TDSEICONV2BIG  = 2404, 
+		TDSEICONV2BIG  = 2404,
+		TDSEPORTINSTANCE	= 2500,
 		TDSESYNC = 20001, 
 		TDSEFCON = 20002, 
 		TDSETIME = 20003, 
Index: freetds83/src/tds/config.c
===================================================================
--- freetds83.orig/src/tds/config.c	2010-01-09 15:33:18.848078805 +0100
+++ freetds83/src/tds/config.c	2010-01-10 00:42:04.821464901 +0100
@@ -630,10 +630,8 @@
 	if (login->block_size) {
 		connection->block_size = login->block_size;
 	}
-	if (login->port) {
+	if (login->port)
 		connection->port = login->port;
-		tds_dstr_copy(&connection->instance_name, "");
-	}
 	if (login->connect_timeout)
 		connection->connect_timeout = login->connect_timeout;
 
@@ -1040,11 +1038,7 @@
 			 * Not set in the [global] section of the
 			 * configure file, take a guess.
 			 */
-#ifdef TDS50
-			ip_port = 4000;
-#else
-			ip_port = 1433;
-#endif
+			ip_port = TDS_DEF_PORT;
 		} else {
 			/*
 			 * Preserve setting from the [global] section
@@ -1091,14 +1085,12 @@
 	if (pSep && pSep != server) {	/* yes, i found it! */
 		/* modify connection-> && login->server_name & ->port */
 		login->port = connection->port = atoi(pSep + 1);
-		tds_dstr_copy(&connection->instance_name, "");
 	} else {
 		/* handle instance name */
 		pSep = strrchr(server, '\\');
 		if (!pSep || pSep == server)
 			return 0;
 
-		login->port = connection->port = 0;
 		tds_dstr_copy(&connection->instance_name, pSep + 1);
 	}
 
Index: freetds83/src/tds/login.c
===================================================================
--- freetds83.orig/src/tds/login.c	2010-01-10 00:42:04.001470742 +0100
+++ freetds83/src/tds/login.c	2010-01-10 00:42:04.831448083 +0100
@@ -427,6 +427,11 @@
 		return TDSECONN;
 	}
 
+	if (connection->port && !tds_dstr_isempty(&connection->instance_name)) {
+		tdserror(tds->tds_ctx, tds, TDSEPORTINSTANCE, 0);
+		return TDSECONN;
+	}
+
 	if (!IS_TDS50(tds) && !tds_dstr_isempty(&connection->instance_name))
 		connection->port = tds7_get_instance_port(tds_dstr_cstr(&connection->ip_addr), tds_dstr_cstr(&connection->instance_name));
 
Index: freetds83/src/tds/mem.c
===================================================================
--- freetds83.orig/src/tds/mem.c	2010-01-09 15:33:18.858049486 +0100
+++ freetds83/src/tds/mem.c	2010-01-10 00:42:04.831448083 +0100
@@ -813,7 +813,6 @@
 	if (!tds_dstr_copy(&connection->server_name, TDS_DEF_SERVER))
 		goto Cleanup;
 	connection->tds_version = TDS_DEFAULT_VERSION;
-	connection->port = TDS_DEF_PORT;
 	connection->block_size = 0;
 	/* TODO use system default ?? */
 	if (!tds_dstr_copy(&connection->client_charset, "ISO-8859-1"))
Index: freetds83/src/tds/util.c
===================================================================
--- freetds83.orig/src/tds/util.c	2010-01-09 15:33:18.868077296 +0100
+++ freetds83/src/tds/util.c	2010-01-10 00:42:04.841460599 +0100
@@ -255,6 +255,7 @@
 	, { TDSEICONVI,      EXCONVERSION,	"Some character(s) could not be converted into client's character set.  "
 						"Unconverted bytes were changed to question marks ('?')" }
 	, { TDSEICONV2BIG,   EXCONVERSION,	"Some character(s) could not be converted into client's character set" }
+	, { TDSEPORTINSTANCE,      EXCOMM,      "Both port and instance specified" }
 	, { TDSERPND,           EXPROGRAM,	"Attempt to initiate a new Adaptive Server operation with results pending" }
 	, { TDSEBTOK,              EXCOMM,	"Bad token from the server: Datastream processing out of sync" }
 	, { TDSECAP,               EXCOMM,	"DB-Library capabilities not accepted by the Server" }