In article <3g************ @individual.net >
Michael Mair <Mi**********@i nvalid.invalid> wrote:
Your design makes the function non-reentrant.
A "snvast(cha r *buf, size_t buflen, char *firststring, ...)"
would cure this problem, apart from being able to return a sensible
error state. vast() could still be used as convenient wrapper to
snvast() for most uses.
Sensible return types for snvast could be int (similar
semantics as snprinf()) or size_t (return strlen(buf)+1 on success
and 0 on failure).
Or even just size_t, with failure "impossible ".
In addition to what everyone else has mentioned, it is worth adding
that any variable-argument function should be defined in "two parts",
as it were.
Consider a function like sprintf(). Note that there is a vsprintf()
function too. Now consider sscanf(); there was no vsscanf() in
C89 but there is one in C99. If you have a system that has syslog()
(either in its "regular" C library or in an add-on library), does
it have vsyslog()? (Some do, some do not; but all *should*.)
The pattern here is clear: if there is a variable-arguments function
f(fixed1, fixed2, var...), eventually someone will need vf(fixed1,
fixed2, va_list). So whenever you write f() you should also write
vf(), and in fact, you should write f() in terms of a call to vf().
I have one last note to add. Concatenating a series of strings
exposes what I consider to be a weakness in the ANSI/ISO C standard
version of <stdarg.h>. If we look at a function like printf() or
fprintf(), we always have at least one, and sometimes several,
fixed arguments:
int printf(const char *, ...);
int fprintf(FILE *, const char *, ...);
In these cases, it is obvious where to put the ", ..." and how
to use the Standard va_start(), because there are one or two
fixed arguments followed by the variable-argument part. When
working with a series of all-identical strings, however, the
"most obvious" prototype -- at least for those of us who start
counting with zero :-) -- is:
char *vast(...);
because the "first" string could be the end-marker, telling us
to concatenate a total of zero strings.
Fortunately, we can "force-fit" the system to work by requiring
at least the end-marker itself, which is also of type "const char *":
char *vast(const char *, ...);
which can legally be called as:
result = vast((/*const*/ char *)NULL); /* const optional */
/* or even just vast(0), since the first argument has a prototype */
Of course, since vast() itself "should be" written in terms of
vvast() -- following the usual rule that requires a vsprintf()
for every sprintf() -- this makes vvast() also require the
one fixed argument before the possibly-empty va_list:
char *vast(const char *str, ...) {
va_list ap;
char *ret;
va_start(ap, str);
ret = vvast(str, ap);
va_end(ap);
return ret;
}
Of course, if you write vast() in terms of snvast(), and instead
of having a vvast() function at all, provide only vsnvast(), the
awkwardness shows. Suppose you had decided initially to write:
size_t snvast(char *buf, size_t bufsize, ...) {
size_t result;
va_list ap;
va_start(ap, bufsize);
result = vsnvast(buf, bufsize, ap);
va_end(ap);
return result;
}
which is, to me, the "natural" form: all the varying arguments
are in the "..." part, even if the very first one is NULL.
Then, for whatever reason, suppose you now decide to add the
static-buffer version called vast():
char *vast(const char *str, ...) {
size_t len;
va_list ap;
static char vastbuf[512]; /* or some other size */
/*
* Awkwardness: handle initial NULL without calling vsnvast()
* at all.
*/
if (str == NULL) {
vastbuf[0] = 0;
return vastbuf;
}
/*
* Since str!=NULL, we have some string(s).
*
* More awkwardness: we have a fixed argument that we
* cannot nudge back into the va_list part. We have to
* handle it ourselves.
*/
va_start(ap, str);
len = strlen(str);
if (len >= sizeof vastbuf) /* 1st string too long: truncate */
len = sizeof vastbuf - 1;
memcpy(vastbuf, len, str);
/*
* Now (sizeof vastbuf - len) >= 1, so snvast() will
* '\0'-terminate the buffer for us in all cases. If
* the first string was short enough, it will also
* concatenate the remaining strings into the rest
* of the buffer.
*
* We ignore snvast()'s return value on purpose here.
*/
snvast(&vastbuf[len], sizeof vastbuf - len, ap);
va_end(ap);
return vastbuf;
}
All of this could have been avoided, had the C89 committee folks
just defined va_start() the way it worked in the old <varargs.h>
header (i.e., without the "last fixed parameter" [parmN] argument).
Then we would just write:
char *vast(...) {
va_list ap;
static char vastbuf[512]; /* or some other size */
va_start(ap); /* XXX -- NOT VALID C89 / C99 */
(void) vsnvast(vastbuf , sizeof vastbuf, ap);
va_end(ap);
return vastbuf;
}
Alas, they did not; and thus if you do write vsnvast(), it might
be best to put a little awkwardness into *it*. Here is the "obvious"
way to write vsnvast():
/*
* Concatenate a series of strings (possibly empty).
* Return the number of bytes that would have been required
* to hold the complete result, including the '\0' terminator.
* (Note that this is strlen(all strings) + 1 and is thus
* a little different from vsnprintf(), which returns what
* strlen(result) would be if the buffer were big enough.)
*
* If size==0, buf may be NULL, and we will not write the
* terminating '\0'.
*/
size_t vsnvast(char *buf, size_t size, va_list ap) {
const char *s;
char *dst = buf;
size_t needed = 1;
size_t spaceleft = size ? size - 1 : 0;
while ((s = va_arg(ap, const char *)) != NULL) {
len = strlen(s);
needed += len;
if (len > spaceleft)
len = spaceleft;
/*
* We should not actually need "if (len)" here at all,
* but the spec for memcpy() was is not too clear about
* allowing NULL if len==0, so might as well test.
*/
if (len) {
memcpy(dst, s, len);
dst += len;
}
}
if (size) /* caller provided at least 1 byte */
*dst = '\0'; /* so terminate the result */
return needed;
}
But if we change the guts of vsnvast() a little bit, so that the
first string is also a fixed parameter, that will simplify vast()
enormously, removing all the sections I marked as awkward. Here
is the "awkward-ized" vsnvast():
/* copy comments from above, please :-) */
size_t vsnvast(char *buf, size_t size, const char *s0, va_list ap) {
const char *s;
char *dst = buf;
size_t needed = 1;
size_t spaceleft = size ? size - 1 : 0;
if ((s = s0) != NULL) {
do {
len = strlen(s);
needed += len;
if (len > spaceleft)
len = spaceleft;
if (len) {
memcpy(dst, s, len);
dst += len;
}
} while ((s = va_arg(ap, const char *)) != NULL);
}
if (size)
*dst = '\0';
return needed;
}
This turns the loop upside down, adding a sequence that really
should not be required (the whole (s = s0) != NULL thing), but
it saves a lot of work in the "simplified " vast() interface,
which now reads:
char *vast(const char *s0, ...) {
va_list ap;
static char vastbuf[512]; /* or some other size */
va_start(ap, s0);
(void) vsnvast(vastbuf , sizeof vastbuf, s0, ap);
va_end(ap);
return vastbuf;
}
(Of course, you have to include the appropriate headers, and
all of the above is quite untested.)
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) +1 801 277 2603
email: forget about it
http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.
