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:
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.
Yeah, well, stupidly my own code had the same problem when I
posted it, plus a bunch of other bogons, which is why my test
did not catch the errors. Silly me.
I will add +1 everywhere for space.
Then you should probably test for overflow, since *printf return
a signed int, and signed integer overflow is UB. It is easier
to do the arithmetic unsigned, as I have done.
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.
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.
A helper function to abstract away some of the boilerplate is
not going to hurt you. See below.
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.
Unfortunately, the C language does not provide any mechanism
beyond the extremely crude notion of "translation units" and
`static` for building namespaces that limit the visibility of
non-local identifiers, and the preprocessor bypasses even that,
so you have to resort to hacky and fragile things like manually
adding text prefaces to have some approximation of the desired
effect. But you have already got one of those, so you may as
well use it.
- Dan C.
Here is a version that avoids nested #if:
/*
* SPDX-License-Identifier: LGPL 2.1 OR Apache 2.0
*
* vasprintf implementation
*/
#include <assert.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include "config.h"
static const size_t FAIL = ~(size_t)0;
static inline int
tds_vsnprintf(char *dst, size_t len, const char *fmt, va_list ap)
{
#ifdef HAVE_VSNPRINTF
return vsnprintf(dst, len, fmt, ap);
#else
assert(dst != NULL);
assert(len != 0);
return vsprintf(dst, fmt, ap);
#endif
}
#ifdef REENTRANT
#define neednull(fp) 1
#define putnull(fp) fclose(fp)
#else
#define neednull(fp) ((fp) == NULL)
#define putnull(fp)
#endif
static inline size_t
tds_vasprintf_bufsize(const char *fmt, va_list ap)
{
int len;
#if defined(HAVE_VSCPRINTF)
len = _vscprintf(fmt, ap);
#elif defined(HAVE_VSNPRINTF)
len = vsnprintf(NULL, 0, fmt, ap);
#else
static FILE *fp = NULL;
if (neednull(fp))
fp = fopen(TDS_PATH_DEVNULL, "w");
if (fp == NULL)
return FAIL;
len = vfprintf(fp, fmt, ap);
putnull(fp);
#endif
if(len < 0)
return FAIL;
return (size_t)len + 1;
}
int
tds_vasprintf(char **outp, const char *fmt, va_list ap)
{
#if defined(HAVE_VASPRINTF)
return vasprintf(outp, fmt, ap);
#elif !defined(va_copy)
#error "va_copy macro required"
#else
va_list save_ap;
va_copy(save_ap, ap);
size_t len = tds_vasprintf_bufsize(fmt, ap);
if (len == FAIL)
return -1;
char *dst = malloc(len);
if (dst == NULL)
return -1;
tds_vsnprintf(dst, len, fmt, save_ap);
*outp = dst;
return (int)len;
#endif
}
#ifdef TEST
#include <assert.h>
#include <string.h>
char *
dovasprintf(const char *fmt, ...)
{
va_list ap;
va_start(ap, fmt);
char *out = NULL;
int rv = tds_vasprintf(&out, fmt, ap);
va_end(ap);
if (rv < 0)
return NULL;
return out;
}
int
main()
{
char *p = dovasprintf("This is the %dth test: %s", 10, "test string");
assert(strcmp(p, "This is the 10th test: test string") == 0);
return EXIT_SUCCESS;
}
#endif