Ex26, strange behavior after assigning to array of strings?

C
#1

I feel like I’m missing something obvious here

in ex26, i’m attempting to read a list of files from list.txt, then store those in an array of strings to read later. However after assigning to the array, when I read the array back it has seemingly changed

I can assign to the array in a while loop, but when i read it back, all values appear to be the last value that was assigned.

A code snippet:

#include <stdio.h>
#include <stdlib.h>
#include <strings.h>

int main(int argc, char *argv[])
{
    int result;
    int errno = 0;

    // array of strings that is the location of files to read
    char *files[4];
    char tempf[512];
    int lineno = 0;

    FILE *fp;
    // list.txt is a file containing a list of other filenames to open
    // file1.txt, file2.txt, file3.txt
    char *fname = "list.txt";
    // open the file, assign to *fp
       if((fp = fopen(fname, "r")) == NULL) {
        printf("cant find file");
        return(-1);
    } 
    // read in the list of files 
    // set tokens to ignore
    // char badtoks[] = "\n\v\x19";
    char badtoks[] = "\n";
    while(fgets(tempf, 512, fp) != NULL) {
        files[lineno] = strtok(tempf, badtoks);
        printf("files[%d] is %s\n", lineno,  files[lineno]);
        lineno++;
    }
    // now the while loops is done so read back the files array
    int z = 0;
    for(z = 0; files[z] != '\0'; z++) {
            printf("file %d is  %s\n", z, files[z]); 
    }
    return 0;
}

The program output:
bryce-e@channel:ex26$./snippet list.txt hello
files[0] is file1.txt
files[1] is file2.txt
files[2] is file3.txt
file 0 is file3.txt
file 1 is file3.txt
file 2 is file3.txt

the contents of list.txt
file1.txt
file2.txt
file3.txt

#2

Your “array of filenames” is a set of pointers, pointing to the same buffer - no matter which one you use, you will read the same contents back. Cheers.

#3

Hmm but inside of the while loop, the values are correct, you can see on the first 3 lines of the program output that they’re “file1.txt, file2.txt, file3.txt”, then after the while loop exits all 3 entries are “file3.txt”

looking at it in gdb I can see that there are 3 values in the *files array, all 3 are file3.txt so they aren’t all pointing at the same place

#4

I see now that every entry in *files[4] ends up pointing the same pointer, which ends up being ‘file3.txt’ since its the last in the loop

I’m unsure of the correct method to create a fixed length array of strings, and append to it as necessary?

I’m able tomake it work with malloc and strncpy, though it seems cognitively strange to me.

while(fgets(tempf, 512, fp) != NULL) {
        char *tempvar = strtok(tempf, badtoks);
        files[lineno] = malloc(512);
        strncpy(files[lineno], tempvar, 512);
        lineno++;
    }
#5

First thing, rewrite this so that you aren’t using strtok. You may think it’s saving you time, but it doesn’t quite work the way you think it does, so that’s causing your error. If you did this manually, you’d not only probably get it right, but also learn a lot about how all of this data is structured.

For example, did you know there’s a strtok_r that works better? What are the return codes of strtok? Why aren’t you handling errors? Does it modify the input string? Do you have to free what it returns?

It would be better if you wrote your own function that scanned for the tokens you wanted and filled in what you wanted.

Next thing, you really need to be using the Zed’s Awesome Debug Macros I have in the book. In case you missed it, they are here https://github.com/zedshaw/liblcthw/blob/master/src/lcthw/dbg.h and will save your bacon like you don’t know. Not only would this solve several of your potential errors, but it’d let you print out variables to debug what’s going on. I can tell right now that you’re kind of just staring at this trying to find out what’s happening, rather than printing. Print everything. EEV-ER-EE-TING!

For example, what happens to your files[] line each loop through? Wouldn’t printing this out be the way you figure this out? All the information you need is right there, you just have to log it and see how the variables or the data are changing while it runs. Remember a bug is not just the code, it’s the data and the execution that matters.

#6

Thanks for the help, I am printing everything, i just removed it from the code snippet to try and improve readability. I was also trying to just get it in a working state before handling errors, but ended up figuring it out by stepping through in gdb, and using %p string format to see what I was doing wrong!

Thanks again, really enjoying the book!

#7

Great, also keep in mind a few other things:

you only have room for lines in the file, but you have a wild loop that will accept input and overwrite all of those. Have you tried slamming this with way more lines?

You’re using strtok and it has some issues so go research what those are. For example it has an internal state that it maintains so if you try to use this across threads it will have huge problems. It also has few security problems based on how it is implemented.

You should also try to write this using a switch statement and see how it compares to what you have there. I think if you did that you would understand the actual memory layout a lot better than if you used the code you have here.

Finally, definitely definitely definitely definitely start using the debug macros. Always use them to check the return values of everything. You’re assuming a lot of return values that just aren’t there and not handling errors that come up. With the debug macros you would’ve caught most of those errors as you recoding and it would’ve been really clear where they came from.