F1e3ab214a976a39cfd713bc93deb10f

whoop

#include <stdio.h>
#include <stdarg.h>
#include <string.h>

void
strcatm(char *str, ...) {
  char *arg;
  va_list args;
  va_start(args, str);
  while (arg = va_arg(args, char *)) strcat(str, arg);
  va_end(args);
}

int 
main() {
  char stmt[100];
  char *a = "foo";
  char *b = "  return foo;";
  strcatm(stmt, "if (!", a, "){\n", b, "\n}");
  puts(stmt);
  
  return 0;
}


# Outputs:

if (!foo){
  return foo;
}

Refactorings

No refactoring yet !

D41d8cd98f00b204e9800998ecf8427e

bob

May 20, 2009, May 20, 2009 18:30, permalink

No rating. Login to rate!

The way you have written strcatm(), you have to explicitly terminate the list of arguments with NULL, so that the while-condition will become false and the loop will terminate. Otherwise it will start reading into random places in memory.

strcatm(stmt, "if (!", a, "){\n", b, "\n}", NULL);
30893470bedd2b0c76d289d157c980db

yoda

May 21, 2009, May 21, 2009 02:40, permalink

No rating. Login to rate!

1. Why not just use sprintf()?
2. Calling strcat() in a loop like that is pretty inefficient. Each call has to scan the increasingly longer string from the beginning. It's an O(n^2) algorithm.

sprintf(stmt, "if (!%s) {\n%s\n}", a, b);
2803bebd594a477aec6f7a7dc218578f

yoda

May 21, 2009, May 21, 2009 02:46, permalink

No rating. Login to rate!

Also, your main() will fail if 'stmt' is not initialized to the empty string, since the first strcat() will concatenate to whatever random bytes are in 'stmt' (whatever garbage is on the stack). So you would need to add this after line 18:
stmt[0] = '\0';

F1e3ab214a976a39cfd713bc93deb10f

Tj Holowaychuk

May 21, 2009, May 21, 2009 03:07, permalink

No rating. Login to rate!

good question, not sure why im not using sprintf haha.. (new to C)

A07f8d0f843a8ec4dad62bc9e87511ba

kulp

October 29, 2009, October 29, 2009 20:49, permalink

No rating. Login to rate!

If you don't like putting the final sentinel in explicitly, and you're using C99 or a compiler that supports vararg macros similarly, you might find it helpful to rename the function and use a macro that adds the sentinel for you. Of course, this brings its own set of problems, but it's sometimes handy.

#define strcatm(...) _strcatm(__VA_ARGS__, NULL)

void
_strcatm(char *str, ...) {
  char *arg;
  va_list args;
  va_start(args, str);
  while (arg = va_arg(args, char *)) strcat(str, arg);
  va_end(args);
}

Your refactoring





Format Copy from initial code

or Cancel