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;
    }
  }
}

Your `point` pointer doesn't point to anything. And, even if it did, you don't have a mechanism by which to return it to the caller of your function. — Adrian Mole, Oct 13 '21 at 14:55
Greg, what is the value of `point`, the first time `*point++ = *str++;` is executed? — chux - Reinstate Monica, Oct 13 '21 at 15:19
You don't need `continue` at the end of a loop. Loops automatically continue unless you break out. — Barmar, Oct 13 '21 at 15:27
You only increment `str` when it points to `$`. In other cases, you keep testing the same character and get an infinite looop. — Barmar, Oct 13 '21 at 15:28
You're only skipping the `$` character, not the character after it. — Barmar, Oct 13 '21 at 15:29

score 0 · Answer 1 · answered Oct 13 '21 at 15:40

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];
    }  
}

frippe · Answer 2 · 2021-10-14T09:27:12.513

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);
}

Cannot assign chars from one string to another

2 Answers2