Search code examples
carraysstringpointersdereference

C: array of pointers pointing to the same value


I'm writing an IP forwarding program, and I need to splice the following routing table into a char* array

  128.15.0.0    255.255.0.0    177.14.23.1
  137.34.0.0    255.255.0.0    206.15.7.2
  137.34.128.0  255.255.192.0  138.27.4.3
  137.34.0.0    255.255.224.0  139.34.12.4
  201.17.34.0   255.255.255.0  192.56.4.5
  27.19.54.0    255.255.255.0  137.7.5.6
  0.0.0.0       0.0.0.0        142.45.9.7

I have initialized an array of pointers to char* like so char *routing[128][128];, then I loop through each line, build a char temp[128] array which then get's assigned to routing as shown below.

In other words, each pointer in the routing array will point to an IP address.

char *routing[128][128];
char line[128], temp[128];
int i = 0, j = 0, token_count = 0, line_count = 0;
while(fgets(line, sizeof(line), routing_table) != NULL){
  // printf("%s\n", line);
  token_count = 0;
  for (i = 0; i < strlen(line); i++){
    if (line[i] != ' ' && token_count == 2){
      for (j = 0; j < 128; j++){
        if (line[i+j] == ' ')
          break;
        temp[j] = line[i+j];
      }
      i += j;
      routing[line_count][token_count] = temp; //ASSIGNMENT OCCURS HERE
      token_count++;
    }
    else if (line[i] != ' ' && token_count == 1){
      for (j = 0; j < 128; j++){
        if (line[i+j] == ' ')
          break;
        temp[j] = line[i+j];
      }
      i += j;
      routing[line_count][token_count] = temp;
      token_count++;
    }
    else if (line[i] != ' ' && token_count == 0){
      for (j = 0; j < 128; j++){
        if (line[i+j] == ' ')
          break;
        temp[j] = line[i+j];
      }
      i += j;
      routing[line_count][token_count] = temp;
      token_count++;
    }
  }
  line_count++;
}

The problem is that every time an assignment happens, it updates the entire array. So when I print out the array at the end, I get the last element repeated.

142.45.9.7

142.45.9.7

142.45.9.7

...

142.45.9.7

142.45.9.7

I figured I needed to dereference temp but that threw a warning and didn't even fix it.

What do I have wrong syntactically? Why are all pointers pointing to the same char string?


Solution

  • Right near the end, just before where you increment token_count, you've got this line:

    routing[line_count][token_count] = temp;
    

    And that's your problem. temp is an array of 128 characters -- what we call a string in C, if there's a null terminator in there somewhere. You're copying each IP address string into it in turn, and then for each one, you assign temp's address to a slot in one of your big two dimensional array of pointers. All those pointers, but they're all pointing to the same actual buffer.

    So at the end of the loop, they all point to the last string you copied into temp. That's not what you want, and if temp is a local in a function, then you're in real trouble.

    What you want is either a two dimensional array of actual char arrays (16 chars each would suffice for a dotted quad and a null terminator) -- but I'm a little rusty on C and won't risk leading you astray about how to declare that -- or else you need to allocate new memory for each string in your existing two dimensional array. You'd do that like so, in many C implementations:

    routing[line_count][token_count] = strdup(temp);
    

    strdup() is a handy function that allocates sufficient memory for the string you give it, copies the string into that memory, and returns a pointer to it. But it's not part of the standard, so can't be counted on in all cases. If you're writing for one compiler and it's there, you're all set (this is often the case). But if you may have any concerns about portability, here's a portable way to do the same thing.

    Portable or not, you're allocating memory now, and when you're done with the routing table, you'll need to free the allocated memory by calling free(routing[i][j]) on every one of those pointers that you got back from strdup() (or from some other, more portable function).

    To that end, you should call this before allocating anything:

    memset(routing, 0, sizeof(char) * 128 * 128);
    

    ...where 128 and 128 are the two dimensions of the array (which I would put in #defined constants that I used for the loop and for the declaration, if I really had to do this in C, and didn't allocate the whole mess dynamically). If you first set the whole thing to zeroes, then every pointer in there starts out as NULL. Then when you loop through it to free the memory, you can just loop through each "row" freeing each pointer until you hit a NULL, and then stop.