String concatenation function, request for comments.

[michael.casey]

The purpose of this post is to obtain the communities opinion of the
usefulness, efficiency, and most importantly the correctness of this
small piece of code. I thank everyone in advance for your time in
considering this post, and for your comments.

I wrote this function to simplify the task of combining strings using
mixed sources. I often see the use of sprintf() which results in
several lines of code, and more processing than really for the task.
The function is pretty straight forward, it accepts a variable list of
arguments beginning with the mandatory integer containing the total
number of arguments.

The function allocates a static buffer; I thought the use of malloc()
to be unnecessary, as the responsibility of freeing the memory would be
left to the client. This design choice also minimizes the amount of
required arguments. If the resultant buffer is needed beyond the second
call to the function, it can be copied to another buffer at the clients
digression.

If the strings specified for concatenation exceed the buffer available,
the overall string will simply be truncated. I decided on this
behaviour because I do not believe the output to be essential to a
programs operation, so this error can in most cases go silent, but
still be apparent to the programmer once the result is examined.

If the defined buffer amount is insufficient or is unnecessarily large
for a particular application, it is left to the client to redefine the
buffer amount; I feel that 512 bytes is sufficient for my use.

I find this function most useful when frequently forming messages for
use over a socket, which will not otherwise be reused.

Example use of the function:

char *tmp;
tmp = vast("Welcome to ", HOST, ", you are logged in as ", name, ".");

send(sockfd, tmp, strlen(tmp), 0);

The function is defined as follows:

/*------------------------------------------------------------------*\
|- vast(): Variable Argument String; concatenate arbitrary strings. -|
\*------------------------------------------------------------------*/

#define VAST_BUF 512

char *vast(int num, ...) {

static char buf[VAST_BUF];

int t = 0,
o = 0;

char *v;

va_list ap;

if(num > 1) {
va_start(ap, num);

/* Loop it while its hot. */

while(num--) {
v = va_arg(ap, char *);

/* Copy string to cumulative buffer until NUL is reached */
/* so long as the total does not exceed (MAX_BUF - 1). */

while((buf[o] = *v++) && !(t >= VAST_BUF - 1)) {
/* Condition is met, increment offset and total. */
t++, o++;
}
}

va_end(ap);
}

buf[o] = NUL;

return(buf);

}

 

[Me]

char *tmp;

tmp = vast("Welcome to ", HOST, ", you are logged in as ", name, ".");

First parameter is an integer not a string. I personally would change
it so you don't have to specify the number of parameters and do:

vast("a", "b", "c", (char*)0);

#define VAST_BUF 512

char *vast(int num, ...) {

I suggest size_t num, but that's just me.

static char buf[VAST_BUF];

int t = 0,
o = 0;

get rid of t.

char *v;

const char * and move this inside the loop

va_list ap;

move this inside the if

if(num > 1) {

if (num > 0)

va_start(ap, num);

/* Loop it while its hot. */

while(num--) {
v = va_arg(ap, char *);

const char *

/* Copy string to cumulative buffer until NUL is reached */
/* so long as the total does not exceed (MAX_BUF - 1). */

What's MAX_BUF?

while((buf[o] = *v++) && !(t >= VAST_BUF - 1)) {
/* Condition is met, increment offset and total. */
t++, o++;
}

Change this to:

while (buf[o] = *v++) {
if (++o >= sizeof(buf)/sizeof(buf[0])-1) {
num = 0;
break;
}
}

}

va_end(ap);
}

buf[o] = NUL;

Just use '\0' or 0.

 

[Paul Mesken]

static char buf[VAST_BUF];

You do realize that using a static buffer will be a possible cause of
bad behaviour as soon as your program does some multitasking? (two
threads could be executing your function at the same time, making a
mess in your buffer).

Such errors have a tendency to be hard to find (since most of the
time, things go as expected and, if the error occurs, it's near
impossible to reproduce it).

Your function is not "re-entrant" (no function using static data is
re-entrant).

I think it would be better and more flexible to pass a pointer to a
destination buffer and an indication of its length.

 

[Eric Sosman]

The purpose of this post is to obtain the communities opinion of the
usefulness, efficiency, and most importantly the correctness of this
small piece of code. I thank everyone in advance for your time in
considering this post, and for your comments.

I wrote this function to simplify the task of combining strings using
mixed sources. I often see the use of sprintf() which results in
several lines of code, and more processing than really for the task.
The function is pretty straight forward, it accepts a variable list of
arguments beginning with the mandatory integer containing the total
number of arguments.

The function allocates a static buffer; I thought the use of malloc()
to be unnecessary, as the responsibility of freeing the memory would be
left to the client. This design choice also minimizes the amount of
required arguments. If the resultant buffer is needed beyond the second
call to the function, it can be copied to another buffer at the clients
digression.

ITYM "discretion" -- but I digress.

If the strings specified for concatenation exceed the buffer available,
the overall string will simply be truncated. I decided on this
behaviour because I do not believe the output to be essential to a
programs operation, so this error can in most cases go silent, but
still be apparent to the programmer once the result is examined.

This strikes me as a poor design choice. Essentially, you are
saying that long concatenated strings are only for "decorative"
purposes, and are "unimportant" to the correct operation of the
program. To put it another way, part of the "contract" for your
function is "Don't use this for anything important." This limits
the function's usefulness (one of the attributes you asked about);
for example, it would be unsafe to use the function to generate
database queries. It would be unsafe even for some uses where a
human reader is involved: Imagine getting a message on your text
pager that said "Emergency! The fertilizer has hit the air
circulator! Drop everything and meet me right away at"

If the defined buffer amount is insufficient or is unnecessarily large
for a particular application, it is left to the client to redefine the
buffer amount; I feel that 512 bytes is sufficient for my use.

I find this function most useful when frequently forming messages for
use over a socket, which will not otherwise be reused.

Example use of the function:

char *tmp;
tmp = vast("Welcome to ", HOST, ", you are logged in as ", name, ".");

This demonstrates a fragility of the interface. It's YOUR
function and YOUR interface design, and in YOUR very first example
of its use you've made an error. When such things happen to me
(nobody's perfect), I start to wonder whether the interface I've
designed should perhaps be redesigned.

Personally, I dislike interfaces that require the coder to count
things -- it dates from my early FORTRAN days, when I had to write
character strings as 13HHELLO, WORLD! and hope that I hadn't blown it.

send(sockfd, tmp, strlen(tmp), 0);

The function is defined as follows:

/*------------------------------------------------------------------*\
|- vast(): Variable Argument String; concatenate arbitrary strings. -|
\*------------------------------------------------------------------*/

#define VAST_BUF 512

char *vast(int num, ...) {

static char buf[VAST_BUF];

int t = 0,
o = 0;

What's the point of maintaining two variables whose values
are always equal?

char *v;

va_list ap;

if(num > 1) {

Peculiar choice: if somebody tries to "concatenate" just one
string (`p = vast(1, "Alpha and Omega")'), he'll get the empty
string in return.

va_start(ap, num);

/* Loop it while its hot. */

while(num--) {
v = va_arg(ap, char *);

/* Copy string to cumulative buffer until NUL is reached */
/* so long as the total does not exceed (MAX_BUF - 1). */

while((buf[o] = *v++) && !(t >= VAST_BUF - 1)) {

Can't you find a more obscure way to write `t < VAST_BUF - 1'?

/* Condition is met, increment offset and total. */
t++, o++;
}
}

va_end(ap);
}

buf[o] = NUL;

NUL has not been defined. Assuming you intend to define it
as zero, the name `NUL' is a misnomer: you do *not* want some
particular character in some particular encoding, you want the
zero-valued character regardless of encoding -- ASCII, EBCDIC,
you name it.

An aside: This assignment is required only if `num' was
less than two to begin with.

return(buf);

}

Summary: I don't like the design -- but that's to some extent
a matter of personal taste about which reasonable people can
disagree. The implementation is all right, once a few missing
pieces like <stdarg.h> and NUL are supplied, but could be made
clearer. The documentation is inadequate; in particular, the value
511 is part of the external interface but is not described anywhere,
and the surprising treatment of single-string "concatenations"
deserves a mention. Usefulness: minimal. Efficiency: unanswerable
in terms of Standard C. Correctness: also unanswerable due to the
lack of a specification.

 

[Michael Mair]

The purpose of this post is to obtain the communities opinion of the
usefulness, efficiency, and most importantly the correctness of this
small piece of code. I thank everyone in advance for your time in
considering this post, and for your comments.

I wrote this function to simplify the task of combining strings using
mixed sources. I often see the use of sprintf() which results in
several lines of code, and more processing than really for the task.
The function is pretty straight forward, it accepts a variable list of
arguments beginning with the mandatory integer containing the total
number of arguments.

The function allocates a static buffer; I thought the use of malloc()
to be unnecessary, as the responsibility of freeing the memory would be
left to the client. This design choice also minimizes the amount of
required arguments. If the resultant buffer is needed beyond the second
call to the function, it can be copied to another buffer at the clients
digression.

If the strings specified for concatenation exceed the buffer available,
the overall string will simply be truncated. I decided on this
behaviour because I do not believe the output to be essential to a
programs operation, so this error can in most cases go silent, but
still be apparent to the programmer once the result is examined.

If the defined buffer amount is insufficient or is unnecessarily large
for a particular application, it is left to the client to redefine the
buffer amount; I feel that 512 bytes is sufficient for my use.

I find this function most useful when frequently forming messages for
use over a socket, which will not otherwise be reused.

Note: Try to be a little bit more short and concise when describing what
your code does or is intended to do -- then people will see what you
want and not miss an important detail.
Your above description could be shortened to your wanting to have sort
of a strncat() with arbitrarily many arguments where the destination
buffer is an internal static buffer. At least an introduction of this
kind might be helpful.

Me's reply (<[email protected]>)
raised most of the important points, so I will more or less
try to make you aware of other stuff I saw.

Example use of the function:

char *tmp;
tmp = vast("Welcome to ", HOST, ", you are logged in as ", name, ".");

Apart from the forgotten number argument and the better design
using a terminating (char *)0 (the cast is necessary for non-fixed
arguments of variadic functions).
You could get into trouble when using const-qualified strings and the
like; on the other hand, using const char* arguments would leave you
with typecasts for everything.

Your design makes the function non-reentrant.
A "snvast(char *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).

send(sockfd, tmp, strlen(tmp), 0);

The function is defined as follows:

/*------------------------------------------------------------------*\
|- vast(): Variable Argument String; concatenate arbitrary strings. -|
\*------------------------------------------------------------------*/

#define VAST_BUF 512

char *vast(int num, ...) {

static char buf[VAST_BUF];

int t = 0,
o = 0;

char *v;

va_list ap;

if(num > 1) {
va_start(ap, num);

/* Loop it while its hot. */

while(num--) {
v = va_arg(ap, char *);

/* Copy string to cumulative buffer until NUL is reached */
/* so long as the total does not exceed (MAX_BUF - 1). */

while((buf[o] = *v++) && !(t >= VAST_BUF - 1)) {
/* Condition is met, increment offset and total. */
t++, o++;
}
}

va_end(ap);
}

buf[o] = NUL;

return(buf);

Minor nit: return is not a function.
return buf;
is entirely sufficient.

}

I agree with most of the points Me raised, so I didn't comment on the
same things.


Cheers
Michael

 

[michael.casey]

Sorry about the mistype of the first actual parameter of vast(), and
the reference to MAX_BUF in my comment where I intended VAST_BUF.

Terminating the variable argument list with a nul ptr to char would
make using the function easier, however I wonder is it valid to use
va_start() without the first named parameter marking the beginning of
the varying portion of the argument list?

I agree with getting rid of 't', it managed to make its way into my
code from a previous version. It is indeed unnecessary.

if (num > 0)

I intended to only accept num if it where > 1, since if a call to
concatenate a single string to nothing where made, it would surely be
an error.
Can you suggest another reason to allow a single argument?

while (buf[o] = *v++) {
if (++o >= sizeof(buf)/sizeof(buf[0])-1) {
num = 0;
break;
}
}

Do you think that it would be more efficient to compute the sizeof(buf)
/ sizeof(buf[0]) on each iteration? And is the division by the
(guaranteed by the standard) char size of 1 necessary?

Is there any reason to use '\0' instead of NUL? - or can it just be
misinterpreted as NULL by other programmers therefore lacks clarity?

 

[michael.casey]

I have learned a lot so far, thank you and everyone.

A thread-safe design is most certainly important here, I will alter my
approach.

static char buf[VAST_BUF];

You do realize that using a static buffer will be a possible cause of
bad behaviour as soon as your program does some multitasking? (two
threads could be executing your function at the same time, making a
mess in your buffer).

Such errors have a tendency to be hard to find (since most of the
time, things go as expected and, if the error occurs, it's near
impossible to reproduce it).

Your function is not "re-entrant" (no function using static data is
re-entrant).

I think it would be better and more flexible to pass a pointer to a
destination buffer and an indication of its length.

 

[michael.casey]

Thank you for your suggestions and the english lesson.

ITYM "discretion" -- but I digress.

I would hope it to be at the clients discretion, yes.

This strikes me as a poor design choice. Essentially, you are
saying that long concatenated strings are only for "decorative"
purposes, and are "unimportant" to the correct operation of the
program. To put it another way, part of the "contract" for your
function is "Don't use this for anything important." This limits
the function's usefulness (one of the attributes you asked about);
for example, it would be unsafe to use the function to generate
database queries. It would be unsafe even for some uses where a
human reader is involved: Imagine getting a message on your text
pager that said "Emergency! The fertilizer has hit the air
circulator! Drop everything and meet me right away at"

Good point. Besides dynamic memory allocation handled by the function,
with the responsibility placed on the client, what alternatives do I
have?

I don't believe it is too much to ask that the client ensure sufficient
space is allocated for the intended string.

This demonstrates a fragility of the interface. It's YOUR
function and YOUR interface design, and in YOUR very first example
of its use you've made an error. When such things happen to me
(nobody's perfect), I start to wonder whether the interface I've
designed should perhaps be redesigned.

A simple oversight. If I did not think this function should be
redesigned I would not have chosen to request comments on it. I believe
everything is subject to redesign.

Personally, I dislike interfaces that require the coder to count
things -- it dates from my early FORTRAN days, when I had to write
character strings as 13HHELLO, WORLD! and hope that I hadn't blown it.

send(sockfd, tmp, strlen(tmp), 0);

The function is defined as follows:

/*------------------------------------------------------------------*\
|- vast(): Variable Argument String; concatenate arbitrary strings. -|
\*------------------------------------------------------------------*/

#define VAST_BUF 512

char *vast(int num, ...) {

static char buf[VAST_BUF];

int t = 0,
o = 0;

What's the point of maintaining two variables whose values
are always equal?

There is no point, I will remove it.

Peculiar choice: if somebody tries to "concatenate" just one
string (`p = vast(1, "Alpha and Omega")'), he'll get the empty
string in return.

My initial thought was to save processing because the function call was
an error to begin with, return an empty string so that it will be most
obvious that the usage of this function was not intended to begin with.

This probably needs some re-thinking.

va_start(ap, num);

/* Loop it while its hot. */

while(num--) {
v = va_arg(ap, char *);

/* Copy string to cumulative buffer until NUL is reached */
/* so long as the total does not exceed (MAX_BUF - 1). */

while((buf[o] = *v++) && !(t >= VAST_BUF - 1)) {

Can't you find a more obscure way to write `t < VAST_BUF - 1'?

I will try.

/* Condition is met, increment offset and total. */
t++, o++;
}
}

va_end(ap);
}

buf[o] = NUL;

NUL has not been defined. Assuming you intend to define it
as zero, the name `NUL' is a misnomer: you do *not* want some
particular character in some particular encoding, you want the
zero-valued character regardless of encoding -- ASCII, EBCDIC,
you name it.

I did not know that NUL was undefined.

An aside: This assignment is required only if `num' was
less than two to begin with.

Good point.

Summary: I don't like the design -- but that's to some extent
a matter of personal taste about which reasonable people can
disagree. The implementation is all right, once a few missing
pieces like <stdarg.h> and NUL are supplied, but could be made
clearer. The documentation is inadequate; in particular, the value
511 is part of the external interface but is not described anywhere,
and the surprising treatment of single-string "concatenations"
deserves a mention. Usefulness: minimal. Efficiency: unanswerable
in terms of Standard C. Correctness: also unanswerable due to the
lack of a specification.

Thank you for your comments.

 

[Chris Torek]

Your design makes the function non-reentrant.
A "snvast(char *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.)

 

[Gregory Pietsch]

This whole thing sounds like another job for FreeDOS Edlin's string
library (available from ibiblio or alt.sources).

The purpose of this post is to obtain the communities opinion of the
usefulness, efficiency, and most importantly the correctness of this
small piece of code. I thank everyone in advance for your time in
considering this post, and for your comments.

I wrote this function to simplify the task of combining strings using
mixed sources. I often see the use of sprintf() which results in
several lines of code, and more processing than really for the task.
The function is pretty straight forward, it accepts a variable list of
arguments beginning with the mandatory integer containing the total
number of arguments.

sprintf()? If you're concatenating strings, I would suggest strcat(),
strncat(), or, if you're using the FreeDOS Edlin string library,
DSappend() or DSappendcstr().

The function allocates a static buffer; I thought the use of malloc()
to be unnecessary, as the responsibility of freeing the memory would be
left to the client. This design choice also minimizes the amount of
required arguments. If the resultant buffer is needed beyond the second
call to the function, it can be copied to another buffer at the clients
digression.

It's better to go whole hog and use properly dynamically-allocated
strings.

If the strings specified for concatenation exceed the buffer available,
the overall string will simply be truncated. I decided on this
behaviour because I do not believe the output to be essential to a
programs operation, so this error can in most cases go silent, but
still be apparent to the programmer once the result is examined.

This wouldn't be acceptable for, say, a GNU utility.

If the defined buffer amount is insufficient or is unnecessarily large
for a particular application, it is left to the client to redefine the
buffer amount; I feel that 512 bytes is sufficient for my use.

Until the 512th character comes....

I find this function most useful when frequently forming messages for
use over a socket, which will not otherwise be reused.

Example use of the function:

char *tmp;
tmp = vast("Welcome to ", HOST, ", you are logged in as ", name, ".");

send(sockfd, tmp, strlen(tmp), 0);

The function is defined as follows:

/*------------------------------------------------------------------*\
|- vast(): Variable Argument String; concatenate arbitrary strings. -|
\*------------------------------------------------------------------*/

#define VAST_BUF 512

char *vast(int num, ...) {

static char buf[VAST_BUF];

int t = 0,
o = 0;

char *v;

va_list ap;

if(num > 1) {
va_start(ap, num);

/* Loop it while its hot. */

while(num--) {
v = va_arg(ap, char *);

/* Copy string to cumulative buffer until NUL is reached */
/* so long as the total does not exceed (MAX_BUF - 1). */

while((buf[o] = *v++) && !(t >= VAST_BUF - 1)) {
/* Condition is met, increment offset and total. */
t++, o++;
}
}

va_end(ap);
}

buf[o] = NUL;

return(buf);

}

Seems too complicated. Let's whittle that down a bit...

#include <string.h>
#include <stdarg.h>
#include "dynstr.h" /* From FreeDOS Edlin */

char *vast(char *x, ...)
{
va_list ap;
static STRING_T *s = 0;
char *p;

if (s == 0)
s = DScreate();
DSassigncstr(s, x, 0);
va_start(ap, x);
while ((p = va_arg(ap, char *)) != 0)
Dsappendcstr(s, p, 0);
va_end(ap);
return DScstr(s);
}

It's probably easier to just return a STRING_T (easier to get the
length).

Gregory Pietsch

 

[Emmanuel Delahaye]

/*------------------------------------------------------------------*\
\*------------------------------------------------------------------*/

#define VAST_BUF 512

char *vast(int num, ...) {

static char buf[VAST_BUF];

return (buf);

Forbidden by QC in a libray (Reentrency issues). Pass the addressof the
array and its size instead. Also, get a better name

char *flexstrcat (char *buf, size_t size, int num, ...) {

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"Clearly your code does not meet the original spec."
"You are sentenced to 30 lashes with a wet noodle."
-- Jerry Coffin in a.l.c.c++

 

[Emmanuel Delahaye]

Me wrote on 05/06/05 :

First parameter is an integer not a string. I personally would change
it so you don't have to specify the number of parameters and do:

You can't. The first parameter of a variadic function must be formal.

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

..sig under repair

 

[Peter Nilsson]

Me wrote on 05/06/05 :

First parameter is an integer not a string. I personally would change
it so you don't have to specify the number of parameters and do:
[vast("a", "b", "c", (char*)0);]

You can't. The first parameter of a variadic function must be formal.

You can...

char *vast(const char *, ...);

 

[michael.casey]

Below is a revision based on the suggestions made. How does this sit
with everyone?

Example"
char vbuf[VAST_BUF];
ret = vast(vbuf, VAST_BUF, "Put it", " together.", EOL)

-

#include <stdarg.h>

#define VAST_BUF 512

/*-------------------------------------------------------------------*\
|- vast(): Variable Argument String; concatenate arbitrary strings. -|
\*-------------------------------------------------------------------*/

/* End of List - Symbolize end of argument list. */

#define EOL (const char *)0

/* Would the const qualifyer be necessary here? */

char *vast(char *buf, size_t len, ...) {

const char *v;
int o = 0;

va_list ap;

va_start(ap, len);

/* Loop it while its hot. */

while(v = va_arg(ap, const char *)) {
/* Copy string to cumulative buffer so long as */
/* the total does not exceed (VAST_BUF - 1). */

while((buf[o] = *v++) && (o < VAST_BUF - 1))
o++;
}

va_end(ap);

buf[o] = '\0';

return(buf);

}

-

I will take the function naming conventions into consideration in the
future.

I have learned quite a few things through this post, and I thank
everyone for their comments.

The purpose of this post is to obtain the communities opinion of the
usefulness, efficiency, and most importantly the correctness of this
small piece of code. I thank everyone in advance for your time in
considering this post, and for your comments.

I wrote this function to simplify the task of combining strings using
mixed sources. I often see the use of sprintf() which results in
several lines of code, and more processing than really for the task.
The function is pretty straight forward, it accepts a variable list of
arguments beginning with the mandatory integer containing the total
number of arguments.

The function allocates a static buffer; I thought the use of malloc()
to be unnecessary, as the responsibility of freeing the memory would be
left to the client. This design choice also minimizes the amount of
required arguments. If the resultant buffer is needed beyond the second
call to the function, it can be copied to another buffer at the clients
digression.

If the strings specified for concatenation exceed the buffer available,
the overall string will simply be truncated. I decided on this
behaviour because I do not believe the output to be essential to a
programs operation, so this error can in most cases go silent, but
still be apparent to the programmer once the result is examined.

If the defined buffer amount is insufficient or is unnecessarily large
for a particular application, it is left to the client to redefine the
buffer amount; I feel that 512 bytes is sufficient for my use.

I find this function most useful when frequently forming messages for
use over a socket, which will not otherwise be reused.

Example use of the function:

char *tmp;
tmp = vast("Welcome to ", HOST, ", you are logged in as ", name, ".");

send(sockfd, tmp, strlen(tmp), 0);

The function is defined as follows:

/*------------------------------------------------------------------*\
|- vast(): Variable Argument String; concatenate arbitrary strings. -|
\*------------------------------------------------------------------*/

#define VAST_BUF 512

char *vast(int num, ...) {

static char buf[VAST_BUF];

int t = 0,
o = 0;

char *v;

va_list ap;

if(num > 1) {
va_start(ap, num);

/* Loop it while its hot. */

while(num--) {
v = va_arg(ap, char *);

/* Copy string to cumulative buffer until NUL is reached */
/* so long as the total does not exceed (MAX_BUF - 1). */

while((buf[o] = *v++) && !(t >= VAST_BUF - 1)) {
/* Condition is met, increment offset and total. */
t++, o++;
}
}

va_end(ap);
}

buf[o] = NUL;

return(buf);

}

 

[Malcolm]

Summary: I don't like the design -- but that's to some extent
a matter of personal taste about which reasonable people can
disagree. The implementation is all right, once a few missing
pieces like <stdarg.h> and NUL are supplied, but could be made
clearer. The documentation is inadequate; in particular, the value
511 is part of the external interface but is not described anywhere,
and the surprising treatment of single-string "concatenations"
deserves a mention. Usefulness: minimal. Efficiency: unanswerable
in terms of Standard C. Correctness: also unanswerable due to the
lack of a specification.

Bascially I agree with most of what Eric Sosman has said.

The function is useful only if an extremly efficient multiple strcat() is
required, and you are prepared to sacrifice thread safety and the ability to
handle arbitrarily long strings for that.
If the string is designed for human consumption then normally a few
microseconds of CPU time doesn't matter, given that the user will spend a
second or so reading the string. However maybe you have a huge number of
users connected at any one time.

 

[Eric Sosman]

Thank you for your suggestions and the english lesson.

Eric Sosman wrote:
[...]

This demonstrates a fragility of the interface. It's YOUR
function and YOUR interface design, and in YOUR very first example
of its use you've made an error. When such things happen to me
(nobody's perfect), I start to wonder whether the interface I've
designed should perhaps be redesigned.

A simple oversight. [...]

Agreed. My point is that if even the inventor of the interface
can make such a simple oversight, the interface may in and of itself
be error-prone. It's at least worth giving thought to what sort of
interface might be less likely to be, er, oversought.

This particular error isn't as bad as it used to be in the pre-
Standard days, because if there's a prototype for vast() in scope
the compiler will catch the omission. The same can't be said for
using a sentinel; the compiler will *not* catch a failure to append
`(char*)NULL' after the "real" arguments. This leaves you with a
balancing act: The existing interface may provoke more coding
errors that the compiler will catch, while a sentinel method
might provoke fewer errors but nastier.

Note that the compiler will not detect the problem in

tmp = vast(4, "Welcome to ", HOST, ", you are logged in as ",
name, ".");

or (worse) in

#define HOST "kremvax"
...
tmp = vast(5, "Welcome to " HOST ", you are logged in as ",
name, ".");

That is, omitting the count is not the only error mode you need to
think about.

 

[CBFalconer]

I have learned a lot so far, thank you and everyone.

A thread-safe design is most certainly important here, I will
alter my approach.

Please don't toppost. Your answer belongs after, or intermixed
with, the material to which you reply, with non-germane stuff
snipped out. That way each article makes sense by itself.

Having found out about re-entrancy, you are also largely
reinventing the wheel. The functions strlcpy and strlcat as
proposed by the BSD group will handle almost everything of interest
to you. You can find an implementation in standard C, together
with documentation and references to the original material, at:

<http://cbfalconer.home.att.net/download/strlcpy.zip>

 

[Me]

First parameter is an integer not a string. I personally would change

You can't. The first parameter of a variadic function must be formal.

char * vast(const char *, ...);

 

[michael.casey]

I am aware of strlcpy(), what I was trying to avoid is multi-line
string concatenations. How does the second version of the function
(posted Jun 5, 4:56 pm) hold up in terms of threading?

 

[michael.casey]

Efficiency was a small part of my goal here. My goals are:

- Avoid redundant multi-line concatenations using strcat() or the like.
- Learn my mistakes in design/implementation by publishing the code.
- Make it efficient to boot.