In article <
10479sq$edc4$1@dont-email.me>,
Arne Vajhøj <
arne@vajhoej.dk> wrote:
On 7/3/2025 9:00 PM, Craig A. Berry wrote:
[snip]
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.
>
I know. But just in case it did get called.
>
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
>
There are duplicated lines.
>
But I still believe the one block of code per case is more readable
than common code + some case specific code + common code + some
case specific code + common code.
Do you have a rationale for that belief, or is it entirely
subjective? It's ok if it is, but best to be up front about it
if so.
Also imagine if someone was to add yet another approach. Now it
is just put in the #if for the case and then write the block of
code. If mixing it would all depend on the new approach - maybe
it would fit into the existing structure, maybe it would require
changing the structure if it does not fit.
Here's an example, with the "try to vsnprintf into a buffer and
just copy if that succeds" approach I mentioned earlier.
There's no nesting of ifdefs, but because the calculation of the
buffer size is decoupled from the logic of allocation and
formatting, the latter is simpler.
- Dan C.
/*
* 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 <string.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)
{
#if defined(HAVE_VSNPRINTF)
return vsnprintf(dst, len, fmt, ap);
#else
assert(dst != NULL);
assert(len != 0);
return vsprintf(dst, fmt, ap);
#endif
}
#if defined(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"
#endif
va_list save_ap;
va_copy(save_ap, ap);
char *dst;
size_t len;
int rv;
#if defined(HAVE_VSNPRINTF)
char buf[1024];
rv = vsnprintf(buf, sizeof(buf), fmt, ap);
if (rv < 0)
return -1;
len = (size_t)rv + 1;
if (len <= sizeof(buf)) {
dst = malloc(len);
if (dst == NULL)
return -1;
memcpy(dst, buf, len);
*outp = dst;
return rv;
}
#else
len = tds_vasprintf_bufsize(fmt, ap);
#endif
if (len == FAIL)
return -1;
dst = malloc(len);
if (dst == NULL)
return -1;
rv = tds_vsnprintf(dst, len, fmt, save_ap);
*outp = dst;
return rv;
}
#if defined(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