Cannot assign chars from one string to another

Question

So I have a function that takes a string, and strips out special format characters, and assigns it to another string for later processing.

A sample call would be:

act_new("$t does $d");

It should strip out the $t and the $d and leave the second string as " does ", but its not assigning anything. I am getting back into programming after quite a few years of inactivity, and this is someone elses code (A MUD codebase, Rom), but I feel like I am missing something fundamental with pointer assignments. Any tips?

(This is truncated code, the rest has no operations on str or point until much later)

void act_new(const char *format)
{
  const char *str;
  char *point;

  str = format;

  while ( *str != '\0' ) {
    if ( *str != '$' ) {
      *point++ = *str++;
      continue;
    }
  }
}

Answer 1

You need to increment str every time through the loop, not only when you assign to point. Otherwise you end up in an infinite loop when the character doesn't match the if condition.

You also want to skip the character after $, so you have to increment str twice when you encounter $.

The code is simpler if you use a for loop and array indexing rather than pointer arithmetic.

size_t len = strlen(format);
for (size_t i = 0; i < len; i++) {
    if (format[i] == '$') {
        i++; // extra increment to skip character after $
    } else {
        *point++ = format[i];
    }  
}

Answer 2

There are a few problems with your code, as pointed out in the comments:

• point is not initialized (garbage pointer value)
• continue doesn't do anything
• infinite loop if a $ is encountered

When writing the function, one must also keep in mind to skip an extra character if a $ is encountered if and only if it's not the last character in the string (except for the '\0').

Since you know how many times you need to loop, a for loop is better suited and, as a bonus, you don't have to explicitly check if the character after a $ is '\0' when skipping an extra character in format (renamed src below). Also, don't forget to terminate the destination string.

This code will take care of those things for you:

void act_new(const char *src)
{
    const size_t length = strlen(src);
    char * const dst = (char*)malloc(sizeof(char)*(length+1));
    if(dst == NULL)
        // Error handling left out
        return;

    char *point = dst;

    for(size_t i = 0; i < length; ++i)
    {
        if(src[i] == '$')
        {
            ++i;
            continue;
        }
        *point++ = src[i];
    }
    *point = '\0';  //Terminate string properly

    printf("%s\n", dst);
    free(dst);
}