On 6/3/2025 12:22 PM, Richard Jordan wrote:
Is there a downloadable build of the FreeTDS package for VSI VMS on
x86 available? I don't have access to a C compiler to try building it.
1.5.2 descrip_mms.template has:
... Craig A. Berry ... 23-JAN-2003
But I could take a look at building on x86-64.
Is there a downloadable build of the FreeTDS package for VSI VMS on x86 available? I don't have access to a C compiler to try building it.
On 6/3/2025 1:27 PM, Arne Vajhøj wrote:
On 6/3/2025 12:22 PM, Richard Jordan wrote:
Is there a downloadable build of the FreeTDS package for VSI VMS on
x86 available? I don't have access to a C compiler to try building it.
1.5.2 descrip_mms.template has:
... Craig A. Berry ... 23-JAN-2003
But I could take a look at building on x86-64.
Did.
* download 1.5.2
* unpack on VMS
* @[.vms]configure
* mmk
* note only I message
* zip tree with -V
* upload
https://www.vajhoej.dk/arne/temp/freetds-1_5_2.zip
Nothing is tested. No guarantees.
(I actually do have some TDS code somewhere that I could test with,
but I assume that you have your test setup ready, so faster that
you get it)
Arne
On 6/3/2025 1:27 PM, Arne Vajhøj wrote:
On 6/3/2025 12:22 PM, Richard Jordan wrote:
Is there a downloadable build of the FreeTDS package for VSI VMS on
x86 available? I don't have access to a C compiler to try building it.
1.5.2 descrip_mms.template has:
... Craig A. Berry ... 23-JAN-2003
But I could take a look at building on x86-64.
Did.
* download 1.5.2
* unpack on VMS
* @[.vms]configure
* mmk
* note only I message
* zip tree with -V
* upload
https://www.vajhoej.dk/arne/temp/freetds-1_5_2.zip
Nothing is tested. No guarantees.
(I actually do have some TDS code somewhere that I could test with,
but I assume that you have your test setup ready, so faster that
you get it)
On 6/3/25 12:45 PM, Arne Vajhøj wrote:
On 6/3/2025 1:27 PM, Arne Vajhøj wrote:
On 6/3/2025 12:22 PM, Richard Jordan wrote:
Is there a downloadable build of the FreeTDS package for VSI VMS on1.5.2 descrip_mms.template has:
x86 available? I don't have access to a C compiler to try building it. >>>
... Craig A. Berry ... 23-JAN-2003
But I could take a look at building on x86-64.
Did.
* download 1.5.2
* unpack on VMS
* @[.vms]configure
* mmk
* note only I message
* zip tree with -V
* upload
https://www.vajhoej.dk/arne/temp/freetds-1_5_2.zip
Nothing is tested. No guarantees.
(I actually do have some TDS code somewhere that I could test with,
but I assume that you have your test setup ready, so faster that
you get it)
It comes with an extensive test suite:
1.) Populate PWD.in with server and credentials
2.) $ define DECC$EFS_CHARSET ENABLE ! for some of the test infrastructure 3.) mmk test
I had it working pretty well on x86 against SQL 2022 a couple of months ago. I think there was one test failure I never finished tracking down.
Updates fail with:
TDS DBLIB error: Unable to allocate sufficient memory
TDS OS error: error 0
TDS error in dbcmd
But I suspect that is a bug in my code.
On 6/5/2025 10:55 AM, Arne Vajhj wrote:
Updates fail with:
TDS DBLIB error: Unable to allocate sufficient memory
TDS OS error: error 0
TDS error in dbcmd
But I suspect that is a bug in my code.
Mysterious.
stat = dbfcmd(con, "INSERT INTO t1 VALUES(%d, '%s')", f1, f2esc);
fails.
char buf[1000];
sprintf(buf, "INSERT INTO t1 VALUES(%d, '%s')", f1, f2esc);
stat = dbcmd(con, buf);
works.
And DBLIB.C contains:
RETCODE
dbfcmd(DBPROCESS * dbproc, const char *fmt, ...)
{
va_list ap;
char *s;
int len;
RETCODE ret;
tdsdump_log(TDS_DBG_FUNC, "dbfcmd(%p, %s, ...)\n", dbproc, fmt);
CHECK_CONN(FAIL);
CHECK_NULP(fmt, "dbfcmd", 2, FAIL);
va_start(ap, fmt);
len = vasprintf(&s, fmt, ap);
va_end(ap);
if (len < 0) {
dbperror(dbproc, SYBEMEM, errno);
return FAIL;
}
ret = dbcmd(dbproc, s);
free(s);
return ret;
}
????
On 2025-06-05, Arne Vajhøj <arne@vajhoej.dk> wrote:
On 6/5/2025 10:55 AM, Arne Vajhøj wrote:
Updates fail with:
TDS DBLIB error: Unable to allocate sufficient memory
TDS OS error: error 0
TDS error in dbcmd
But I suspect that is a bug in my code.
Mysterious.
stat = dbfcmd(con, "INSERT INTO t1 VALUES(%d, '%s')", f1, f2esc);
fails.
char buf[1000];
sprintf(buf, "INSERT INTO t1 VALUES(%d, '%s')", f1, f2esc);
stat = dbcmd(con, buf);
works.
And DBLIB.C contains:
RETCODE
dbfcmd(DBPROCESS * dbproc, const char *fmt, ...)
{
va_list ap;
char *s;
int len;
RETCODE ret;
tdsdump_log(TDS_DBG_FUNC, "dbfcmd(%p, %s, ...)\n", dbproc, fmt);
CHECK_CONN(FAIL);
CHECK_NULP(fmt, "dbfcmd", 2, FAIL);
va_start(ap, fmt);
len = vasprintf(&s, fmt, ap);
va_end(ap);
if (len < 0) {
dbperror(dbproc, SYBEMEM, errno);
return FAIL;
}
ret = dbcmd(dbproc, s);
free(s);
return ret;
}
????
Does this _exact_ code work on Alpha ?
If yes, try building your test code with compiler optimisation disabled.
If that doesn't work, try building the library itself with compiler optimisation disabled.
32-bit or 64-bit build ?
Any compiler build warnings either in the library or your test code ?
(Try setting warnings to fatal to abort the build if you get a lot of
output during building).
I am trying to eliminate the possibility you may have stumbled across
a compiler bug.
Simon.
On 6/5/25 12:33 PM, Simon Clubley wrote:
On 2025-06-05, Arne Vajhøj <arne@vajhoej.dk> wrote:
Mysterious.
stat = dbfcmd(con, "INSERT INTO t1 VALUES(%d, '%s')", f1, f2esc);
fails.
char buf[1000];
sprintf(buf, "INSERT INTO t1 VALUES(%d, '%s')", f1, f2esc);
stat = dbcmd(con, buf);
works.
And DBLIB.C contains:
RETCODE
dbfcmd(DBPROCESS * dbproc, const char *fmt, ...)
{
va_start(ap, fmt);
len = vasprintf(&s, fmt, ap);
va_end(ap);
Does this _exact_ code work on Alpha ?
If yes, try building your test code with compiler optimisation disabled.
If that doesn't work, try building the library itself with compiler
optimisation disabled.
$ mmk clean
$ @[.vms]configure
$ mmk/MACRO=__DEBUG__=1
Any compiler build warnings either in the library or your test code ?
(Try setting warnings to fatal to abort the build if you get a lot of
output during building).
I am trying to eliminate the possibility you may have stumbled across
a compiler bug.
On 6/5/2025 5:16 PM, Craig A. Berry wrote:
On 6/5/25 12:33 PM, Simon Clubley wrote:
On 2025-06-05, Arne Vajhøj <arne@vajhoej.dk> wrote:
Mysterious.
stat = dbfcmd(con, "INSERT INTO t1 VALUES(%d, '%s')", f1, f2esc);
fails.
char buf[1000];
sprintf(buf, "INSERT INTO t1 VALUES(%d, '%s')", f1, f2esc);
stat = dbcmd(con, buf);
works.
And DBLIB.C contains:
RETCODE
dbfcmd(DBPROCESS * dbproc, const char *fmt, ...)
{
va_start(ap, fmt);
len = vasprintf(&s, fmt, ap);
va_end(ap);
Does this _exact_ code work on Alpha ?
If yes, try building your test code with compiler optimisation disabled. >>>
If that doesn't work, try building the library itself with compiler
optimisation disabled.
$ mmk clean
$ @[.vms]configure
$ mmk/MACRO=__DEBUG__=1
It is vasprintf that return -1.
It happens with both normal build and /DEB/NOOPT.
I have not tried on Alpha yet.
Any compiler build warnings either in the library or your test code ?
(Try setting warnings to fatal to abort the build if you get a lot of
output during building).
There are just a few %CC-I-QUESTCOMPARE.
I am trying to eliminate the possibility you may have stumbled across
a compiler bug.
It is a mystery.
I have a query that use dbfcmd and it works. But it fails for
my updates.
I created a standalone example doing the same just without any FreeTDS.
It works fine.
$ typ z.c
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdarg.h>
void dbfcmd(void *dbproc, const char *fmt, ...)
{
va_list ap;
char *s;
int len;
va_start(ap, fmt);
len = vasprintf(&s, fmt, ap);
va_end(ap);
printf("len=%d s=|%s|\n", len, s);
free(s);
}
int main()
{
dbfcmd(NULL, "INSERT INTO t1 VALUES(%d, '%s')", 999, "XXX");
dbfcmd(NULL, "INSERT INTO t1 VALUES(%d, '%s')", 999, "XXX");
dbfcmd(NULL, "INSERT INTO t1 VALUES(%d, '%s')", 999, "XXXXXX");
return 0;
}
$ r z
len=33 s=|INSERT INTO t1 VALUES(999, 'XXX')|
len=33 s=|INSERT INTO t1 VALUES(999, 'XXX')|
len=36 s=|INSERT INTO t1 VALUES(999, 'XXXXXX')|
I must be missing something.
On 6/5/25 8:35 PM, Arne Vajhøj wrote:
On 6/5/2025 5:16 PM, Craig A. Berry wrote:
On 6/5/25 12:33 PM, Simon Clubley wrote:
On 2025-06-05, Arne Vajhøj <arne@vajhoej.dk> wrote:
And DBLIB.C contains:
RETCODE
dbfcmd(DBPROCESS * dbproc, const char *fmt, ...)
{
va_start(ap, fmt);
len = vasprintf(&s, fmt, ap);
va_end(ap);
It is vasprintf that return -1.
It happens with both normal build and /DEB/NOOPT.
I created a standalone example doing the same just without any FreeTDS.
It works fine.
I must be missing something.
Check for the line in your generated descrip.mms that starts with:
VASPRINTFOBJ =
If the right-hand side is empty, you are using the system-supplied vasprintf, which is what should happen. If it's not empty, you are
getting a replacement vasprintf() from FreeTDS, which in turn means the detection code in configure.com is not working right (unless you are on
a version of VMS before vasprintf() was added to the CRTL). Either
should work in principle, but it could be one difference between your standalone test and your FreeTDS test.
On 6/6/2025 8:54 AM, Craig A. Berry wrote:
On 6/5/25 8:35 PM, Arne Vajhøj wrote:
On 6/5/2025 5:16 PM, Craig A. Berry wrote:
On 6/5/25 12:33 PM, Simon Clubley wrote:
On 2025-06-05, Arne Vajhøj <arne@vajhoej.dk> wrote:
And DBLIB.C contains:
RETCODE
dbfcmd(DBPROCESS * dbproc, const char *fmt, ...)
{
va_start(ap, fmt);
len = vasprintf(&s, fmt, ap);
va_end(ap);
It is vasprintf that return -1.
It happens with both normal build and /DEB/NOOPT.
I created a standalone example doing the same just without any FreeTDS.
It works fine.
I must be missing something.
Check for the line in your generated descrip.mms that starts with:
VASPRINTFOBJ =
If the right-hand side is empty, you are using the system-supplied vasprintf, which is what should happen. If it's not empty, you are getting a replacement vasprintf() from FreeTDS, which in turn means the detection code in configure.com is not working right (unless you are on
a version of VMS before vasprintf() was added to the CRTL). Either should work in principle, but it could be one difference between your standalone test and your FreeTDS test.
Bingo.
$ diff descrip.mms
************
File DKA0:[arne.freetds.freetds-1_5_2]descrip.mms;2
149 VASPRINTFOBJ =
150 STRTOK_ROBJ =
******
File DKA0:[arne.freetds.freetds-1_5_2]descrip.mms;1
149 VASPRINTFOBJ = [.src.replacements]vasprintf$(OBJ),
150 STRTOK_ROBJ =
************
$ diff [.include]config.h
************
File DKA0:[arne.freetds.freetds-1_5_2.include]config.h;2
234 #define HAVE_VASPRINTF 1
235
******
File DKA0:[arne.freetds.freetds-1_5_2.include]config.h;1
234 #define HAVE_VASPRINTF 0
235
************
And now dbfcmd works.
Of course it does not explain why the replacement doesn't work.
On 6/6/25 9:42 AM, Arne Vajhøj wrote:
And now dbfcmd works.
Of course it does not explain why the replacement doesn't work.
Nor why the detection code doesn't correctly identify that vasprintf is present. That detection code has been there unchanged for 23 years:
https://github.com/FreeTDS/freetds/blob/ a381342bbfccafc0aa9ed2376e38470907d53225/vms/configure.com#L267
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
int main()
{
char *ptr;
vasprintf(&ptr,"%d,%d",1,2);
exit(0);
}
But it isn't using va_start/va_end, which it surely should. My guess is
this worked by accident on pre-x86 but something tightened up on x86
that makes the vasprintf call fail and causes FreeTDS to use its
fallback implementation. I'm without access at the moment so can't test this theory out myself.
On x86-64:
$ typ zzz.c
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
int main()
{
char *ptr;
vasprintf(&ptr,"%d,%d",1,2);
exit(0);
}
$ cc zzz
vasprintf(&ptr,"%d,%d",1,2);
.......................^
%CC-W-CVTDIFTYPES, In this statement, "1" of type "int", is being
converted to "long pointer to char".
at line number 7 in file DKA0:[arne.freetds]zzz.c;1
vasprintf(&ptr,"%d,%d",1,2);
^
%CC-E-TOOMANYARGS, In this statement, "vasprintf" expects 3 arguments,
but 4 are supplied.
at line number 7 in file DKA0:[arne.freetds]zzz.c;1
On 6/3/2025 1:27 PM, Arne Vajhøj wrote:
On 6/3/2025 12:22 PM, Richard Jordan wrote:
Is there a downloadable build of the FreeTDS package for VSI VMS on
x86 available? I don't have access to a C compiler to try building it.
1.5.2 descrip_mms.template has:
... Craig A. Berry ... 23-JAN-2003
But I could take a look at building on x86-64.
Did.
* download 1.5.2
* unpack on VMS
* @[.vms]configure
* mmk
* note only I message
* zip tree with -V
* upload
https://www.vajhoej.dk/arne/temp/freetds-1_5_2.zip
$ diff descrip.mms
On Sat, 14 Jun 2025 14:09:04 -0400, Arne Vajhøj wrote:
$ diff descrip.mms
Is there a “diff -u” option? I ask because unified diffs are the common format for exchanging source patches in a format that patch(1) (and other compatible tools) will accept.
On 6/14/2025 7:39 PM, Lawrence D'Oliveiro wrote:
On Sat, 14 Jun 2025 14:09:04 -0400, Arne Vajhøj wrote:
$ diff descrip.mms
Is there a “diff -u” option? I ask because unified diffs are the common >> format for exchanging source patches in a format that patch(1) (and
other compatible tools) will accept.
$ DIFF /SLP and $ EDIT /SUM
or grab one of the many ports of *nix diff and patch.
On Sat, 14 Jun 2025 20:38:47 -0400, Arne Vajhøj wrote:
On 6/14/2025 7:39 PM, Lawrence D'Oliveiro wrote:
On Sat, 14 Jun 2025 14:09:04 -0400, Arne Vajhøj wrote:
$ diff descrip.mms
Is there a “diff -u” option? I ask because unified diffs are the common >>> format for exchanging source patches in a format that patch(1) (and
other compatible tools) will accept.
$ DIFF /SLP and $ EDIT /SUM
But those generate precise editor commands, do they not, complete with
line numbers that must match in order for the patch to work.
or grab one of the many ports of *nix diff and patch.
You mean, they’re not standard equipment?
The clever thing about the patch(1) tool is, it can apply a patch to a
file that has already had other patches applied to it: if all they’ve done is move the target area a few lines forward or backward, the patch command will still succeed.
Context/unified diff & patch are smart.
But if someone want them then they can get them.
For command line utilities then I will suggest:
http://antinode.info/dec/sw/diffutils.html
On Sun, 15 Jun 2025 20:22:13 -0400, Arne Vajhøj wrote:
Context/unified diff & patch are smart.
But if someone want them then they can get them.
diff()1 and patch(1) are pretty fundamental tools for open-source collaboration. They should be part of some core installation package,
like Python.
For command line utilities then I will suggest:
http://antinode.info/dec/sw/diffutils.html
The newest version appears to be 3.8. Checking the upstream site <https://www.gnu.org/software/diffutils/>, this is nearly four years
old.
On 6/6/2025 11:18 AM, Arne Vajhøj wrote:
On x86-64:
$ typ zzz.c
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
int main()
{
char *ptr;
vasprintf(&ptr,"%d,%d",1,2);
exit(0);
}
$ cc zzz
vasprintf(&ptr,"%d,%d",1,2);
.......................^
%CC-W-CVTDIFTYPES, In this statement, "1" of type "int", is being
converted to "long pointer to char".
at line number 7 in file DKA0:[arne.freetds]zzz.c;1
vasprintf(&ptr,"%d,%d",1,2);
^
%CC-E-TOOMANYARGS, In this statement, "vasprintf" expects 3 arguments,
but 4 are supplied.
at line number 7 in file DKA0:[arne.freetds]zzz.c;1
On Alpha:
$ typ zzz.c
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
int main()
{
char *ptr;
vasprintf(&ptr,"%d,%d",1,2);
exit(0);
}
$ cc zzz
vasprintf(&ptr,"%d,%d",1,2);
^
%CC-I-IMPLICITFUNC, In this statement, the identifier "vasprintf" is implicitly declared as a function.
at line number 7 in file DISK2:[ARNE]zzz.c;1
$ link zzz
%LINK-W-NUDFSYMS, 1 undefined symbol:
%LINK-I-UDFSYM, VASPRINTF
%LINK-W-USEUNDEF, undefined symbol VASPRINTF referenced
in psect $LINK$ offset %X00000030
in module ZZZ file DISK2:[ARNE]zzz.OBJ;1
I haven't had time yet to dig into why the replacement wasn't working.
Unfortunately that means that the entire fallback algorithm
in tds_vasprintf (write to null device to find output size,
allocate buffer of correct size and then write to buffer)
is broken.
On Mon, 23 Jun 2025 20:17:57 -0400, Arne Vajhøj wrote:
Unfortunately that means that the entire fallback algorithm
in tds_vasprintf (write to null device to find output size,
allocate buffer of correct size and then write to buffer)
is broken.
What a dumb way to do it. Using the null device means making OS I/O calls
-- needless overhead just to find out how long a string -- which is being
put together entirely in userland -- needs to be.
On 6/23/2025 8:37 PM, Lawrence D'Oliveiro wrote:
What a dumb way to do it. Using the null device means making OS I/O
calls -- needless overhead just to find out how long a string -- which
is being put together entirely in userland -- needs to be.
But if you have a simple better way of achieving it ...
On 6/23/2025 8:37 PM, Lawrence D'Oliveiro wrote:
On Mon, 23 Jun 2025 20:17:57 -0400, Arne Vajhøj wrote:
Unfortunately that means that the entire fallback algorithm
in tds_vasprintf (write to null device to find output size,
allocate buffer of correct size and then write to buffer)
is broken.
What a dumb way to do it. Using the null device means making OS I/O calls
-- needless overhead just to find out how long a string -- which is being
put together entirely in userland -- needs to be.
Fallback algorithms are often primitive.
But if you have a simple better way of achieving it then please send
the code to Craig so he can send it upstream.
On Mon, 23 Jun 2025 20:40:21 -0400, Arne Vajhøj wrote:
On 6/23/2025 8:37 PM, Lawrence D'Oliveiro wrote:
What a dumb way to do it. Using the null device means making OS I/O
calls -- needless overhead just to find out how long a string -- which
is being put together entirely in userland -- needs to be.
But if you have a simple better way of achieving it ...
snprintf(3) and vsnprintf(3) return the total number of bytes needed for
the final result. So just call them once with all the right arguments and
a zero-length buffer, and use the result to allocate a buffer of the right length, and call them again.
And of course you should be using va_end(3) and va_start(3) again before
the second call. This allows for traversing the argument list multiple
times.
On 6/23/2025 9:49 PM, Craig A. Berry wrote:
This one looks pretty simple and depends only on vsnprintf, which has
been in VMS since 7.3-2, so it was probably in some ancient standard,
though I haven't looked yet:
https://github.com/jkaivo/asprintf/blob/asprintf/asprintf.h
It's MIT which I think can be included in a GPL project like FreeTDS. I
will try to remember to look at this when I get some other things sorted
out.
It could definitely be used.
But why not just steal the trick from it instead of the whole code?
The trick they use is va_copy.
I think tds_vasprintf could use va_copy to solve the problem as well!
On 6/23/2025 9:49 PM, Craig A. Berry wrote:
This one looks pretty simple and depends only on vsnprintf, which has
been in VMS since 7.3-2, so it was probably in some ancient standard,
though I haven't looked yet:
https://github.com/jkaivo/asprintf/blob/asprintf/asprintf.h
It's MIT which I think can be included in a GPL project like FreeTDS. I
will try to remember to look at this when I get some other things sorted
out.
It could definitely be used.
But why not just steal the trick from it instead of the whole code?
The trick they use is va_copy.
I think tds_vasprintf could use va_copy to solve the problem as well!
This one looks pretty simple and depends only on vsnprintf, which has
been in VMS since 7.3-2, so it was probably in some ancient standard,
though I haven't looked yet:
https://github.com/jkaivo/asprintf/blob/asprintf/asprintf.h
It's MIT which I think can be included in a GPL project like FreeTDS. I
will try to remember to look at this when I get some other things sorted
out.
But the argument list for vasprintf is as it is.
On 6/23/2025 10:16 PM, Arne Vajhøj wrote:
On 6/23/2025 9:49 PM, Craig A. Berry wrote:
This one looks pretty simple and depends only on vsnprintf, which has
been in VMS since 7.3-2, so it was probably in some ancient standard,
though I haven't looked yet:
https://github.com/jkaivo/asprintf/blob/asprintf/asprintf.h
It's MIT which I think can be included in a GPL project like FreeTDS. I
will try to remember to look at this when I get some other things sorted >>> out.
It could definitely be used.
But why not just steal the trick from it instead of the whole code?
The trick they use is va_copy.
I think tds_vasprintf could use va_copy to solve the problem as well!
On the other side, then ditching existing code and taking this
would also switch form vfprintf to vsnprintf for length
determination, which Lawrence is correct would be
better.
On 6/23/25 9:35 PM, Arne Vajhøj wrote:
On 6/23/2025 10:16 PM, Arne Vajhøj wrote:
But why not just steal the trick from it instead of the whole code?
The trick they use is va_copy.
I think tds_vasprintf could use va_copy to solve the problem as well!
On the other side, then ditching existing code and taking this
would also switch form vfprintf to vsnprintf for length
determination, which Lawrence is correct would be
better.
It turns out tds_vasprtintf already uses vsnprintf if HAVE_VSNPRINTF is defined and only falls back to the /dev/null hack if not:
https://github.com/FreeTDS/freetds/blob/master/src/replacements/vasprintf.c
So my next change to FreeTDS will look something like this:
--- vms/config_h.vms;-0 Tue Sep 26 09:23:25 2023
+++ vms/config_h.vms Tue Jun 24 05:23:20 2025
@@ -237,7 +237,9 @@
#define HAVE_SNPRINTF @D_SNPRINTF@
/* Define to 1 if you have the `vsnprintf' function. */
-#define HAVE_VSNPRINTF 0
+#if __CRTL_VER >= 70302000
+#define HAVE_VSNPRINTF 1
+#endif
/* Define as const if the declaration of iconv() needs const. */
#if HAVE_ICONV
[end_of_patch]
That should cover everybody with a VMS system from recent decades.
I don't like the #elif HAVE_VSNPRINTF code in tds_vasprintf.
:-)
The algorithm is basically to try vsnprintf with buffers
increasing in size 512, 1024, 1536, ... until there is space.
That is very inefficient compared to just use the return value.
And since it does not use va_copy, then only the first call to
vsnprintf will work.
On 6/24/2025 12:25 PM, Arne Vajhøj wrote:
I don't like the #elif HAVE_VSNPRINTF code in tds_vasprintf.
:-)
The algorithm is basically to try vsnprintf with buffers
increasing in size 512, 1024, 1536, ... until there is space.
That is very inefficient compared to just use the return value.
And since it does not use va_copy, then only the first call to
vsnprintf will work.
Even if va_copy was called and we ignore the option of
using vsnprintf return value, then +512 is not good
compared to *2. +512 with a 1 GB CLOB value would mean
2 million calls to vsnprintf.
(I have not checked if TDS actually allow that long
statements, but +512 is a problem)
The algorithm is basically to try vsnprintf with buffers increasing in
size 512, 1024, 1536, ... until there is space.
That is very inefficient compared to just use the return value.
On Tue, 24 Jun 2025 12:25:50 -0400, Arne Vajhøj wrote:
The algorithm is basically to try vsnprintf with buffers increasing in
size 512, 1024, 1536, ... until there is space.
That is very inefficient compared to just use the return value.
It’s a good choice in the absence of any other clues as to how much to allocate. I have used it myself. (Of course having an explicit number returned is best of all.)
One may conduct statistical analyses, I suppose to decide on the optimum multiplication factor. E.g. would going up in powers of 3 better than
powers of 2? I suspect larger factors (too far from e = 2.71828 ...) would not be worth using.
And don’t forget to do a realloc(3) at the end, to trim the unused part of the last allocation.
On 6/24/25 7:38 PM, Arne Vajhøj wrote:
On 6/24/2025 12:25 PM, Arne Vajhøj wrote:
I don't like the #elif HAVE_VSNPRINTF code in tds_vasprintf.
:-)
The algorithm is basically to try vsnprintf with buffers
increasing in size 512, 1024, 1536, ... until there is space.
That is very inefficient compared to just use the return value.
And since it does not use va_copy, then only the first call to
vsnprintf will work.
Even if va_copy was called and we ignore the option of
using vsnprintf return value, then +512 is not good
compared to *2. +512 with a 1 GB CLOB value would mean
2 million calls to vsnprintf.
(I have not checked if TDS actually allow that long
statements, but +512 is a problem)
TDS packet length is negotiated but must be between 512 and 32,767 bytes:
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/ c1cddd03-b448-470a-946a-9b1b908f27a7
And vasprintf appears to be used in FreeTDS only for query strings and commands, not for data.
You aren't wrong, but improving the vasprintf implementation isn't my
highest priority;
feel free to take a crack at it and submit a PR. To
work against current sources, you either need to use autoconf with a git checkout or get a nightly snapshot from <https://www.freetds.org>.
Submitting a PR may be a problem. I have never had a personal Github
account and I suspect that my work Github account has been
deactivated.
On 6/25/2025 8:15 AM, Craig A. Berry wrote:
feel free to take a crack at it and submit a PR. To
work against current sources, you either need to use autoconf with a git
checkout or get a nightly snapshot from <https://www.freetds.org>.
I may take a look.
Submitting a PR may be a problem. I have never had a personal
Github account and I suspect that my work Github account has
been deactivated. But I will worry about that when/if I have
a good fix.
In article <68669c25$0$690$14726298@news.sunsite.dk>,
Arne Vajhøj <arne@vajhoej.dk> wrote:
On 7/1/2025 3:57 PM, Arne Vajhøj wrote:
On 6/25/2025 8:15 AM, Craig A. Berry wrote:
feel free to take a crack at it and submit a PR. To
work against current sources, you either need to use autoconf with a git >>>> checkout or get a nightly snapshot from <https://www.freetds.org>.
I may take a look.
Submitting a PR may be a problem. I have never had a personal
Github account and I suspect that my work Github account has
been deactivated. But I will worry about that when/if I have
a good fix.
First take:
https://www.vajhoej.dk/arne/temp/tds_vasprintf.c
Total rewrite. I did not like the old code at all.
Your code is incorrect.
Note that `vsnprintf` et al do not include space for the NUL
terminator in their return value, and you do not account for
this when you malloc your destination buffer.
As the main logic is the same in each case (find out the buffer
size, allocate, and then assign and return), and the only real
difference is in finding out the buffer length, your code would
be more readable with some helper functions that you delegated
to.
You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.
On 7/1/2025 3:57 PM, Arne Vajhøj wrote:
On 6/25/2025 8:15 AM, Craig A. Berry wrote:
feel free to take a crack at it and submit a PR. To
work against current sources, you either need to use autoconf with a git >>> checkout or get a nightly snapshot from <https://www.freetds.org>.
I may take a look.
Submitting a PR may be a problem. I have never had a personal
Github account and I suspect that my work Github account has
been deactivated. But I will worry about that when/if I have
a good fix.
First take:
https://www.vajhoej.dk/arne/temp/tds_vasprintf.c
Total rewrite. I did not like the old code at all.
In article <1046hc5$62th$2@dont-email.me>,
Arne Vajhøj <arne@vajhoej.dk> wrote:
On 7/3/2025 1:40 PM, Dan Cross wrote:
You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.
This must support non *nix systems (as an example
VMS !!) - config.h is expected to provide that with the rest.
If copyright dates are anything to judge by, `<paths.h>` has
been a thing since 1989, but I was wrong in that it is not
actually in POSIX: it is a BSD extension, though common.
In general, it is good practice and good hygiene in C programs
to `#include` the header files that are documented to define the
symbols and types that you use in your program, instead of
relying on transitive includes to populate things ambiantly via
e.g. `config.h`.
Regardless, in that case, you shouldn't use `_PATH_DEVNULL`.
Note the leading `_`: that signifies a reserved identifier,
which is not something you should be defining yourself. If you
want to punt this to e.g. config.h, better would be to define a
new name (say, `TDS_PATH_DEVNULL`) and use that.
On 7/3/2025 1:40 PM, Dan Cross wrote:
In article <68669c25$0$690$14726298@news.sunsite.dk>,
Arne Vajhøj <arne@vajhoej.dk> wrote:
On 7/1/2025 3:57 PM, Arne Vajhøj wrote:
On 6/25/2025 8:15 AM, Craig A. Berry wrote:
feel free to take a crack at it and submit a PR. To
work against current sources, you either need to use autoconf with a git >>>>> checkout or get a nightly snapshot from <https://www.freetds.org>.
I may take a look.
Submitting a PR may be a problem. I have never had a personal
Github account and I suspect that my work Github account has
been deactivated. But I will worry about that when/if I have
a good fix.
First take:
https://www.vajhoej.dk/arne/temp/tds_vasprintf.c
Total rewrite. I did not like the old code at all.
Your code is incorrect.
Note that `vsnprintf` et al do not include space for the NUL
terminator in their return value, and you do not account for
this when you malloc your destination buffer.
Ouch.
I will add +1 everywhere for space.
Thanks.
As the main logic is the same in each case (find out the buffer
size, allocate, and then assign and return), and the only real
difference is in finding out the buffer length, your code would
be more readable with some helper functions that you delegated
to.
As states in the comments then it was a design goal to have
simple code blocks - no nested if's, no functions - for a given
context just 1-10 lines of code.
You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.
This must support non *nix systems (as an example
VMS !!) - config.h is expected to provide that with the rest.
You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.
This must support non *nix systems (as an example VMS !!) - config.h is expected to provide that with the rest.
In article <1046hc5$62th$2@dont-email.me>,
Arne Vajhøj <arne@vajhoej.dk> wrote:
On 7/3/2025 1:40 PM, Dan Cross wrote:
As the main logic is the same in each case (find out the buffer
size, allocate, and then assign and return), and the only real
difference is in finding out the buffer length, your code would
be more readable with some helper functions that you delegated
to.
As states in the comments then it was a design goal to have
simple code blocks - no nested if's, no functions - for a given
context just 1-10 lines of code.
That's a bad design goal.
This sort of organization leads to lots of duplication and tends
to produce code that is overly verbose and fragile, where one
section may be updated and the others skipped or missed. A
little bit of duplication for the sake of comprehensibility is
ok, but this is excessive.
You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.
This must support non *nix systems (as an example
VMS !!) - config.h is expected to provide that with the rest.
If copyright dates are anything to judge by, `<paths.h>` has
been a thing since 1989, but I was wrong in that it is not
actually in POSIX: it is a BSD extension, though common.
In general, it is good practice and good hygiene in C programs
to `#include` the header files that are documented to define the
symbols and types that you use in your program, instead of
relying on transitive includes to populate things ambiantly via
e.g. `config.h`.
Regardless, in that case, you shouldn't use `_PATH_DEVNULL`.
Note the leading `_`: that signifies a reserved identifier,
which is not something you should be defining yourself. If you
want to punt this to e.g. config.h, better would be to define a
new name (say, `TDS_PATH_DEVNULL`) and use that.
https://github.com/FreeTDS/freetds/blob/Branch-1_5/src/replacements/ vasprintf.c
does the following:
#if HAVE_PATHS_H
#include <paths.h>
#endif /* HAVE_PATHS_H */
. . .
#ifndef _PATH_DEVNULL
#define _PATH_DEVNULL "/dev/null"
#endif
FreeTDS is relying on autoconf to sort out whether paths.h is available
and will only include it if HAVE_PATHS_H is defined in config.h, and
thus only defines _PATH_DEVNULL if it's known not to exist. Obviously
one should avoid making up one's own reserved identifiers and clobbering documented and well-known names, but that's not really the same thing as providing an implementation for one that's known to be missing. This
fallback approach could of course be added to Arne's implementation if desired; to me it seems a lot cleaner than defining yet another macro,
which would need to be defined in terms of _PATH_DEVNULL when it exists
but otherwise not.
The problem for VMS is that the workaround doesn't work. Or at least it didn't when I was first porting FreeTDS 20+ years ago because the CRTL
at the time did not recognize '/dev/null' as a valid path. I believe recentish CRTLs (maybe VMS 8.x and later?) do have special case code
that translates it to the native null device. But that wasn't
available, so I added the following line to the template from which
config.h is generated on VMS:
#define _PATH_DEVNULL "_NLA0:"
The main advantage of this approach is that it worked, and it did so
with a one-line change to a file I was already maintaining. I didn't
have to sprinkle '#ifdef __VMS' in files that already existed and were already working on other platforms. I didn't have to create a lot of new files, the presence of which in the repository I couldn't test without autoconf/automake, which I didn't have access to at the time. All of
which meant upstream maintainers were far more likely to accept my changes.
On 7/3/2025 6:31 PM, Craig A. Berry wrote:
https://github.com/FreeTDS/freetds/blob/Branch-1_5/src/replacements/
vasprintf.c
does the following:
#if HAVE_PATHS_H
#include <paths.h>
#endif /* HAVE_PATHS_H */
. . .
#ifndef _PATH_DEVNULL
#define _PATH_DEVNULL "/dev/null"
#endif
FreeTDS is relying on autoconf to sort out whether paths.h is available
and will only include it if HAVE_PATHS_H is defined in config.h, and
thus only defines _PATH_DEVNULL if it's known not to exist. Obviously
one should avoid making up one's own reserved identifiers and clobbering
documented and well-known names, but that's not really the same thing as
providing an implementation for one that's known to be missing. This
fallback approach could of course be added to Arne's implementation if
desired; to me it seems a lot cleaner than defining yet another macro,
which would need to be defined in terms of _PATH_DEVNULL when it exists
but otherwise not.
The problem for VMS is that the workaround doesn't work. Or at least it
didn't when I was first porting FreeTDS 20+ years ago because the CRTL
at the time did not recognize '/dev/null' as a valid path. I believe
recentish CRTLs (maybe VMS 8.x and later?) do have special case code
that translates it to the native null device. But that wasn't
available, so I added the following line to the template from which
config.h is generated on VMS:
#define _PATH_DEVNULL "_NLA0:"
The main advantage of this approach is that it worked, and it did so
with a one-line change to a file I was already maintaining. I didn't
have to sprinkle '#ifdef __VMS' in files that already existed and were
already working on other platforms. I didn't have to create a lot of new
files, the presence of which in the repository I couldn't test without
autoconf/automake, which I didn't have access to at the time. All of
which meant upstream maintainers were far more likely to accept my
changes.
So you are saying that for the code to work on *nix then I need
to keep those, because having it defined in config.h is a non-*nix
thing and for *nix it only set HAVE_PATHS_H which requires the
include in this file?
I will update.
On 7/3/25 7:24 PM, Arne Vajhøj wrote:
On 7/3/2025 6:31 PM, Craig A. Berry wrote:
https://github.com/FreeTDS/freetds/blob/Branch-1_5/src/replacements/
vasprintf.c
does the following:
#if HAVE_PATHS_H
#include <paths.h>
#endif /* HAVE_PATHS_H */
. . .
#ifndef _PATH_DEVNULL
#define _PATH_DEVNULL "/dev/null"
#endif
FreeTDS is relying on autoconf to sort out whether paths.h is available
and will only include it if HAVE_PATHS_H is defined in config.h, and
thus only defines _PATH_DEVNULL if it's known not to exist. Obviously
one should avoid making up one's own reserved identifiers and clobbering >>> documented and well-known names, but that's not really the same thing as >>> providing an implementation for one that's known to be missing. This
fallback approach could of course be added to Arne's implementation if
desired; to me it seems a lot cleaner than defining yet another macro,
which would need to be defined in terms of _PATH_DEVNULL when it exists
but otherwise not.
The problem for VMS is that the workaround doesn't work. Or at least it >>> didn't when I was first porting FreeTDS 20+ years ago because the CRTL
at the time did not recognize '/dev/null' as a valid path. I believe
recentish CRTLs (maybe VMS 8.x and later?) do have special case code
that translates it to the native null device. But that wasn't
available, so I added the following line to the template from which
config.h is generated on VMS:
#define _PATH_DEVNULL "_NLA0:"
The main advantage of this approach is that it worked, and it did so
with a one-line change to a file I was already maintaining. I didn't
have to sprinkle '#ifdef __VMS' in files that already existed and were
already working on other platforms. I didn't have to create a lot of new >>> files, the presence of which in the repository I couldn't test without
autoconf/automake, which I didn't have access to at the time. All of
which meant upstream maintainers were far more likely to accept my
changes.
So you are saying that for the code to work on *nix then I need
to keep those, because having it defined in config.h is a non-*nix
thing and for *nix it only set HAVE_PATHS_H which requires the
include in this file?
I will update.
I think preserving the HAVE_PATHS_H check is probably a good idea. It
makes sure you get the correct, system-specific version of
_PATH_DEVNULL, if any. And it preserves an existing approach to
portability that you aren't worried about so you can focus on the
vasprintf() guts.
I haven't done any testing of your code. I do agree with Dan that
reducing the repeated code would be a benefit. You don't need the first
#if because the file won't even be built if HAS_VASPRINTF is detected
during configuration.
Which means for the remaining 4 cases you've got
this in all of them that could be put outside the #ifdefs, and without
any nesting:
va_list cp;
va_copy(cp, ap);
<something1>
(len < 0) return len;
<something2>
*ret = malloc(len + 1);
if(!*ret) return -1;
return vsprintf(*ret, fmt, cp);
where something1 is always 1-3 lines and something2 is zero or one line
On Thu, 3 Jul 2025 14:15:33 -0400, Arne Vajhøj wrote:
You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.
This must support non *nix systems (as an example VMS !!) - config.h is
expected to provide that with the rest.
Also, there is no paths.h in POSIX.
It doesn’t make any sense to have it
in POSIX.
On 7/3/2025 8:51 PM, Lawrence D'Oliveiro wrote:
It doesn’t make any sense to have it in POSIX.
Given that it is common on POSIX/SUS OS then someone could have thrown
it into the standard. They just didn't.
On 7/3/25 3:07 PM, Dan Cross wrote:
In article <1046hc5$62th$2@dont-email.me>,
Arne Vajhøj <arne@vajhoej.dk> wrote:
On 7/3/2025 1:40 PM, Dan Cross wrote:
You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.
This must support non *nix systems (as an example
VMS !!) - config.h is expected to provide that with the rest.
If copyright dates are anything to judge by, `<paths.h>` has
been a thing since 1989, but I was wrong in that it is not
actually in POSIX: it is a BSD extension, though common.
In general, it is good practice and good hygiene in C programs
to `#include` the header files that are documented to define the
symbols and types that you use in your program, instead of
relying on transitive includes to populate things ambiantly via
e.g. `config.h`.
Regardless, in that case, you shouldn't use `_PATH_DEVNULL`.
Note the leading `_`: that signifies a reserved identifier,
which is not something you should be defining yourself. If you
want to punt this to e.g. config.h, better would be to define a
new name (say, `TDS_PATH_DEVNULL`) and use that.
The code ya'll are talking about replacing:
https://github.com/FreeTDS/freetds/blob/Branch-1_5/src/replacements/vasprintf.c
does the following:
#if HAVE_PATHS_H
#include <paths.h>
#endif /* HAVE_PATHS_H */
. . .
#ifndef _PATH_DEVNULL
#define _PATH_DEVNULL "/dev/null"
#endif
FreeTDS is relying on autoconf to sort out whether paths.h is available
and will only include it if HAVE_PATHS_H is defined in config.h, and
thus only defines _PATH_DEVNULL if it's known not to exist. Obviously
one should avoid making up one's own reserved identifiers and clobbering >documented and well-known names, but that's not really the same thing as >providing an implementation for one that's known to be missing. This
fallback approach could of course be added to Arne's implementation if >desired; to me it seems a lot cleaner than defining yet another macro,
which would need to be defined in terms of _PATH_DEVNULL when it exists
but otherwise not.
The problem for VMS is that the workaround doesn't work. Or at least it >didn't when I was first porting FreeTDS 20+ years ago because the CRTL
at the time did not recognize '/dev/null' as a valid path. I believe >recentish CRTLs (maybe VMS 8.x and later?) do have special case code
that translates it to the native null device. But that wasn't
available, so I added the following line to the template from which
config.h is generated on VMS:
#define _PATH_DEVNULL "_NLA0:"
The main advantage of this approach is that it worked, and it did so
with a one-line change to a file I was already maintaining. I didn't
have to sprinkle '#ifdef __VMS' in files that already existed and were >already working on other platforms. I didn't have to create a lot of new >files, the presence of which in the repository I couldn't test without >autoconf/automake, which I didn't have access to at the time. All of
which meant upstream maintainers were far more likely to accept my changes.
I'm sure I'll get a lecture about everything I did wrong, but at least
now the context is out there for why it was reasonable for Arne to
depend on prior art, i.e., the presence of _PATH_DEVNULL in config.h.
On 7/3/2025 4:07 PM, Dan Cross wrote:
In article <1046hc5$62th$2@dont-email.me>,
Arne Vajhøj <arne@vajhoej.dk> wrote:
On 7/3/2025 1:40 PM, Dan Cross wrote:
As the main logic is the same in each case (find out the buffer
size, allocate, and then assign and return), and the only real
difference is in finding out the buffer length, your code would
be more readable with some helper functions that you delegated
to.
As states in the comments then it was a design goal to have
simple code blocks - no nested if's, no functions - for a given
context just 1-10 lines of code.
That's a bad design goal.
This sort of organization leads to lots of duplication and tends
to produce code that is overly verbose and fragile, where one
section may be updated and the others skipped or missed. A
little bit of duplication for the sake of comprehensibility is
ok, but this is excessive.
It is very easy to read this way.
Just scan sequentially down until one find the relevant
#if context and then read 1-10 lines sequentially.
No jumping up and down.
You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.
This must support non *nix systems (as an example
VMS !!) - config.h is expected to provide that with the rest.
If copyright dates are anything to judge by, `<paths.h>` has
been a thing since 1989, but I was wrong in that it is not
actually in POSIX: it is a BSD extension, though common.
This code should also work on non-*nix.
It does not matter if all or most *nix has had it since 1989.
In general, it is good practice and good hygiene in C programs
to `#include` the header files that are documented to define the
symbols and types that you use in your program, instead of
relying on transitive includes to populate things ambiantly via
e.g. `config.h`.
Regardless, in that case, you shouldn't use `_PATH_DEVNULL`.
Note the leading `_`: that signifies a reserved identifier,
which is not something you should be defining yourself. If you
want to punt this to e.g. config.h, better would be to define a
new name (say, `TDS_PATH_DEVNULL`) and use that.
This has to fit into an existing project that has its
way to setup things.
I also think that its way makes sense. It would not be good
to start putting OS specific includes/defines into lots
of C files. That is what the configure process is supposed to
handle.
| Sysop: | Keyop |
|---|---|
| Location: | Huddersfield, West Yorkshire, UK |
| Users: | 505 |
| Nodes: | 16 (2 / 14) |
| Uptime: | 49:10:25 |
| Calls: | 9,921 |
| Calls today: | 8 |
| Files: | 13,801 |
| Messages: | 6,347,558 |