Re: [PATCH] VMS build update

Frediano Ziglio <[email protected]>
Newsgroups gmane.comp.db.tds.freetds
Message-ID <CAHt6W4eEnZFBVhXxPmLZZ7883uLVj=TuCcY_ZgsdUQMaeDN=Uw@mail.gmail.com>
2013/10/15 Craig A. Berry <[email protected]>:
>
> On Oct 14, 2013, at 12:57 AM, Frediano Ziglio <[email protected]> wrote:
>
>> 2013/10/13 Craig A. Berry <[email protected]>:
>>> Make the VMS-specific configuration and build procedures handle threads, plus admit to having some other things that we do have but weren't configuring for, such as clock_gettime(), socketpair(), getaddrinfo(), and stdio locking.
>>>
>>> The attached patch was created with git format-patch and should be applicable with git am.  Or GNU patch -p1 for traditionalists.
>>>
>>
>> What can I say. Pushed!
>
> Many thanks.
>
>>> 6e3afa4d (Frediano Ziglio        2013-06-05 12:47:50 +0100 1127)        i = fgetc(hostfile);
>>> 6e3afa4d (Frediano Ziglio        2013-06-05 12:47:50 +0100 1128)        if (i == EOF)
>>> 6e3afa4d (Frediano Ziglio        2013-06-05 12:47:50 +0100 1129)                return _bcp_check_eof(dbproc, hostfile, 0);
>>> 6e3afa4d (Frediano Ziglio        2013-06-05 12:47:50 +0100 1130)        ungetc(i, hostfile);
>>>
>>
>> Well... this patch use some stdio extension to make read faster... but
>> fails on standard C calls ?? What a crazy world! It just try to detect
>> if we are at end of the file without removing a character!
>
>
> What you're doing is completely reasonable, but appears to trigger a pretty nasty bug in the C run-time where calling fgetc/ungetc corrupts the return value of a subsequent fread.  Since FreeTDS always checks the return value of fread in _bcp_read_hostfile, it knows something is bonkers and bails out.
>
> I've documented what goes wrong with a simple reproducer at <https://sourceforge.net/p/vms-ports/tickets/72/>.  Not sure what to do yet as a workaround.  I guess we could skip the above if it's purely an optimization.  Or I could try to implement my own ungetc by fiddling with the pointers in the stdio struct.
>

412737? Where this value came from?

Can you try the attached patch? It worked for me (or at least it
didn't break anything).

Frediano

_______________________________________________
FreeTDS mailing list
[email protected]
http://lists.ibiblio.org/mailman/listinfo/freetds
0001-Detect-end-of-file-not-using-ungetc.patch (application/octet-stream, 2.9 KB)
From 25989b61d02b008d92697d221046471e070830aa Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <[email protected]>
Date: Tue, 15 Oct 2013 20:45:17 +0100
Subject: [PATCH] Detect end-of-file not using ungetc

It seems that VMS have problems with this function.

Signed-off-by: Frediano Ziglio <[email protected]>
---
 src/dblib/bcp.c |   15 +++++----------
 src/tds/bulk.c  |   13 ++++++++++---
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/dblib/bcp.c b/src/dblib/bcp.c
index 7925fbe..45d4801 100644
--- a/src/dblib/bcp.c
+++ b/src/dblib/bcp.c
@@ -1119,16 +1119,6 @@ _bcp_read_hostfile(DBPROCESS * dbproc, FILE * hostfile, int *row_error)
 	assert(hostfile);
 	assert(row_error);
 
-	/*
-	 * If we read no bytes and we're at end of file AND this is the first column,
-	 * then we've stumbled across the finish line.  Tell the caller we failed to read
-	 * anything but encountered no error.
-	 */
-	i = fgetc(hostfile);
-	if (i == EOF)
-		return _bcp_check_eof(dbproc, hostfile, 0);
-	ungetc(i, hostfile);
-
 	/* for each host file column defined by calls to bcp_colfmt */
 
 	for (i = 0; i < dbproc->hostfileinfo->host_colcount; i++) {
@@ -1241,6 +1231,11 @@ _bcp_read_hostfile(DBPROCESS * dbproc, FILE * hostfile, int *row_error)
 				return FAIL;
 			}
 
+			if (conv_res == TDS_NO_MORE_RESULTS) {
+				free(coldata);
+				return _bcp_check_eof(dbproc, hostfile, i);
+			}
+
 			if (col_bytes > 0x7fffffffl) {
 				*row_error = TRUE;
 				tdsdump_log(TDS_DBG_FUNC, "data from file is too large!\n");
diff --git a/src/tds/bulk.c b/src/tds/bulk.c
index 966ccde..0309672 100644
--- a/src/tds/bulk.c
+++ b/src/tds/bulk.c
@@ -1001,7 +1001,7 @@ tds_file_stream_read(TDSINSTREAM *stream, void *ptr, size_t len)
 	TDSFILESTREAM *s = (TDSFILESTREAM *) stream;
 	int c;
 	char *p = (char *) ptr;
-#define GETC() do { c = getc_unlocked(s->f); if (c==EOF) return -1; } while(0)
+#define GETC() do { c = getc_unlocked(s->f); if (c==EOF) goto check_eof; } while(0)
 
 	while (s->left_len < s->term_len) {
 		GETC();
@@ -1021,11 +1021,18 @@ tds_file_stream_read(TDSINSTREAM *stream, void *ptr, size_t len)
 		s->left[s->term_len-1] = c;
 	}
 	return p - (char *) ptr;
+
+check_eof:
+	if (!s->left_len && feof_unlocked(s->f))
+		return 0;
+	return -1;
 }
 
 /**
  * Read a data file, passing the data through iconv().
- * \return TDS_SUCCESS or TDS_FAIL.
+ * \retval TDS_SUCCESS  success
+ * \retval TDS_FAIL     error reading the column
+ * \retval TDS_NO_MORE_RESULTS end of file detected
  */
 TDSRET
 tds_bcp_fread(TDSSOCKET * tds, TDSICONV * char_conv, FILE * stream, const char *terminator, size_t term_len, char **outbuf, size_t * outbytes)
@@ -1070,7 +1077,7 @@ tds_bcp_fread(TDSSOCKET * tds, TDSICONV * char_conv, FILE * stream, const char *
 	((char *) w.stream.buffer)[0] = 0;
 	w.stream.write(&w.stream, 1);
 
-	return res;
+	return r.left_len ? res : TDS_NO_MORE_RESULTS;
 }
 
 TDSRET
-- 
1.7.9.5
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.