Re: String concatenation function, request for comments.

michael.casey@xxxxxxxxx wrote: 
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 (<1117978823.695019.91800@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>)
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
--
E-Mail: Mine is an   /at/ gmx /dot/ de   address.
.

Relevant Pages

  • Re: String concatenation function, request for comments.
    ... >> left to the client. ... If the resultant buffer is needed beyond the second ... >> If the strings specified for concatenation exceed the buffer available, ... > This demonstrates a fragility of the interface. ...
    (comp.lang.c)
  • String concatenation function, request for comments.
    ... usefulness, efficiency, and most importantly the correctness of this ... The function allocates a static buffer; I thought the use of malloc ... If the defined buffer amount is insufficient or is unnecessarily large ... it is left to the client to redefine the ...
    (comp.lang.c)
  • Re: [Lit.] Buffer overruns
    ... >> As has been pointed out, C strings are unrelated to C buffer overflow ... > is a infrastructure source length available based on nul-terminated, ... > data-pattern based length paradigm ... ...
    (sci.crypt)
  • Re: null terminated strings
    ... terminated strings are blamed because they were being used in the programs ... involved with the overrun situation. ... maximum length for that buffer and make sure you don't exceed it. ... Languages such as COBOL would automaically pad or truncate fields during ...
    (comp.os.vms)
  • Re: String concatenation function, request for comments.
    ... saying that long concatenated strings are only for "decorative" purposes, and are "unimportant" to the correct operation of the program. ... This limits the function's usefulness; for example, it would be unsafe to use the function to generate database queries. ... When such things happen to me, I start to wonder whether the interface I've designed should perhaps be redesigned. ... Efficiency: unanswerable in terms of Standard C. Correctness: also unanswerable due to the lack of a specification. ...
    (comp.lang.c)
Loading