0

I have something like this (without the spacing issues), where I'm reading a file with strings that are separated by the # sign, and trying to read each of them into an array of char *s. However, each char* that holds a string much be the exact length of the string. This is a little buggy though.

char * fragments[1000];
 char temp[20];
      while (i < numFrags) {
          if (fscanf(fp, "#%15[^#]#", temp) == 1) {
            char entry[strlen(temp)];
        entry = strcpy(entry, temp);
        fragments[i++] = entry;
        printf("%d\n", strlen(temp));

          }
      }

Thoughts?

nmichaels
  • 49,466
  • 12
  • 107
  • 135
  • 1
    "a little buggy" is not useful. Please tell us what the bugs actually are, as in what you put in, what you expected, and what you got out. – David Thornley Apr 08 '11 at 17:23
  • How can a char * be "the exact length of the string"? Or do you mean that each char * must point to one of a collection of strings of equal length? – Pete Wilson Apr 08 '11 at 17:24
  • I think that "a little buggy" is the OP's metaphor: each string, you see, holds characters that it carries around, much as a little buggy or a little red wagon might. I could be wrong. – Pete Wilson Apr 08 '11 at 17:28
  • I'm not sure if I exactly know what you would like to get out of your function, but one quick fix to your problem would be to use dynamic allocation over the heap using malloc and free, instead of using arrays with fixed sizes. – Pirooz Apr 08 '11 at 17:29
  • One problem is allocating the array `entry` locally, then keeping its address. I don't know if the use of `fscanf` is correct, either, since I don't know the input format. – David Thornley Apr 08 '11 at 17:33

1 Answers1

3

Your problem is that you are reusing the entry array.

Look at how for all i we have fragments[i++] = entry;. In the end, every fragment will be pointing to the same place and thus will contain the same string (in this case, it will be the last string read).

It seems as if you tried to work around this by using both a temp and an entry array, but in the end you might just as well have made the fscanf write directly to the entry array.


Another problem that you can get is if your pointers point to an array that was automaticaly allocated on the stack (ie.: an array declared inside a function). These memory locations are reused by your program after your function returns, your pointers can start pointing to invalid data (and since this is C, using these corrupted strings can lead to all sorts of crashes and bad program behaviour).


You problem is basically a memory allocation issue, so there are many solutions.

  1. Make fragments be an array of character arrays (instead of only pointers). This way you safely have a different piece of memory for each string.

    char fragments[2000][20]; //can hold 2000 19-character strings
    for(i=0; i<N; i++){
        scanf("%d", fragments[i]);
    }
    

    The advantage for this solution is that it is simple, and that all memory is allocated beforehand.

  2. Use dynamic memory allocation with malloc. This allows you to choose a different size of array for each string and also allows you to choose the size of your arrays at run time instead of compile time.

    char* fragments[2000];
    char entry[MAX_FRAGMENT_LENGTH]; //temporary buffer for strings
    for(i=0; i<N; i++){
        scanf("%d", entry[i]);
        //dynamically allocate an array, just big enough to fit our string in.
        fragments[i] = malloc(sizeof(char) * (1 + strlen(entry));
        strcpy(fragments[i], entry);
    }
    

    The main advantage for this method is that it is very flexible, while still being reasonably simple. The main disavantage is that you will need to free all pointers that have been malloc-ed after you are done with them (otherwise you can get a memory leak).

  3. Do "dynamic allocation" by hand, using a single large buffer to contain all the strings.

    char buffer[2000*20];
    char* fragments[2000];
    char* next_empty_location = buffer;
    for(i=0; i<N; i++){
        scanf("%d", next_empty_location);
        fragments[i] = next_empty_location;
        next_empty_location += strlen(next_empty_location) + 1;
    }
    

    If you can't/don't want/aren't allowed to use the malloc solution, this is what comes closest. It might be harder to understand (in case you are having issues with C), but is the one that best seems to fit the vague "each char* that holds a string much be the exact length of the string" restriction.

hugomg
  • 68,213
  • 24
  • 160
  • 246