2

I'm trying to assign values from an array containing all strings to an array of structs, where some of the struct members are integers. The way I've attempted it causes some undefined behavior, though. Like the code below generates the following:

0 surname ▒▒
1 forename &
2 id 0

when it should say

0 surname Boatswain
1 forename Michael Jr
2 id 109993267

I'm not exactly sure what's wrong with the way I've assigned these values though.

Code:

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

//There are 7 values for each student
#define VALUES 7

struct Students{
  int term;
  int id; // NEED TO GENERATE ERROR IF WRONG NUM OF CHARACTERS
  char surname[15];
  char forename[15];
  char subject[3];
  int catnum;
  char section[3];
};

int main(int argc, char *argv[]){

//Get contents of input file
  unsigned char filename;
  printf("Enter the input file name: \n");
  scanf("%s", &filename);
  FILE *file = fopen(&filename, "r");
  int filesize=0;
  fseek(file, 0L, SEEK_END);
  filesize = ftell(file);
  fseek(file, 0L, SEEK_SET);
  char *contents = malloc(filesize+1);
  size_t size=fread(contents,1,filesize,file);
  contents[size]=0;  // what does all this do exactly?

//Get number of lines in input file
  int total_line = 0;
  const char *str;
  for(str = contents; *str; ++str)
  total_line += *str == '\n';

//Tokenize string
  int n=0,nn;
  char *b[VALUES*total_line];
  char *ds=strdup(contents);
  b[n]=strtok(ds, ",=\"\r\n\"");
  while(b[n] && n<((VALUES*total_line)-1)) b[++n]=strtok(NULL, ",=\"\r\n\"");
  //for(nn=0; nn<=n; ++nn) printf("%s %d   ", b[nn], nn);
  //putchar('\n');
  free(ds);

  struct Students record[total_line];

  int i, j;
  for(i=0;i++;i<total_line){
    for(j=0;j++;j<VALUES){
      int n=(7*i)+j;
      record[i].term=atoi(b[n]);
      record[i].id=atoi(b[n]);
      strcpy(record[i].surname, b[n]);
      strcpy(record[i].forename, b[n]);
      strcpy(record[i].subject, b[n]);
      record[i].catnum=atoi(b[n]);
      strcpy(record[i].section, b[n]);
    }
  }

// try printing some values here
  printf("0 surname %s\n",record[0].surname); 
  printf("1 forename %s\n",record[1].forename);
  printf("2 id %d\n", record[2].id);

  free(contents);
  return 0;
}

Input file:

1301,107515018,"Boatswain","Michael R.",CSE, 230,="R01"
1301,109993269,"Castille","Michael Jr",CSE, 230,="R03"
1301,109993267,"Castille","Janice",CSE, 230,="R03"

Thank you in advance for the help!

Edit: What is the issue in changing the for loop like so?

for(i=0;i<total_line;i++) { 
  record[i].term=atoi(b[(7*i)]);
  record[i].id=atoi(b[(7*i)+1]);
  strcpy(record[i].surname, b[(7*i)+2]);
  strcpy(record[i].forename, b[(7*i)+3]);
  strcpy(record[i].subject, b[(7*i)+4]);
  record[i].catnum=atoi(b[(7*i)+5]);
  strcpy(record[i].section, b[(7*i)+6]);
}

Edit: I got it to work. Just needed to change the for-loop to a while-loop. Edit2: ohh I had the i++ and i total_line switched. derp. fixed now. Well I guess that solves this mystery.

2
  • One key idea of programming: abstraction. That means hiding complex stuff behind simple descriptive names. The (main) tool therefore in C is the function. Try to decompose your problem into smaller subproblems. Commented Mar 6, 2016 at 19:00
  • 1
    unsigned char filename; means one character. You then write scanf("%s", &filename); which reads many characters . This causes a buffer overflow with unpredictable consequences. Commented Mar 7, 2016 at 21:26

2 Answers 2

2

In your for(j=0;j++;j<VALUES) loop you are converting the same b[n] multiple times. You most likely don't need this loop but to convert b[n++] instead.

Sign up to request clarification or add additional context in comments.

4 Comments

Thanks for the help! I'm confused why this loop wouldn't work though. 0-6 should go to record[0], so then 7*i = 0 and j adds from 0-6 onto it, giving record[0] 0-6, and so on for record[1] getting 7-13, and record[2] getting 14-20. At least, that's what I thought it did. I also don't understand the syntax you're recommending. Commenting out the j-loop and changing the b[n]'s to b[n++] (like record[i].term=atoi(b[n++]);) does not work.
What I would do in a situation like this is to start displaying debug stuff on the screen. For example, and do something like this for every statement in the loop: record[i].term=atoi(b[n]); printf("Stored integer value %d in record[i=%d].term\n",atoi(b[n]),i); That should make it clear what is going on.
ohh pssh. now that I see it I just feel silly. Thanks again for the help!
I tried doing it like this now and it still isn't working... for(i=0;i++;i<total_line){ record[i].term=atoi(b[(7*i)]); record[i].id=atoi(b[(7*i)+1]); strcpy(record[i].surname, b[(7*i)+2]); strcpy(record[i].forename, b[(7*i)+3]); strcpy(record[i].subject, b[(7*i)+4]); record[i].catnum=atoi(b[(7*i)+5]); strcpy(record[i].section, b[(7*i)+6]); } and for some reason printf() isn't causing anything to display..
2

As a preliminary, this line (which apparently you already find suspicious) ...

  contents[size]=0;  // what does all this do exactly?

... adds a string terminator after the bytes read from the file, so the whole thing can safely be interpreted as one big C string.

@DavetheSax makes a valid observation about the indexes of your tokens. That problem does not cause undefined behavior, but the resulting behavior is surely not what you wanted.

You indeed do have undefined behavior in your program, however. You fill array b by duping contents into ds, and then tokenizing ds with strtok(). That in itself is ok, but needful only if you need to preserve contents as it was originally read, which you probably don't. After the tokenization, however, you free ds. That, again, is not a problem in itself, but it invalidates all the token pointers you just computed, because they point into the memory previously allocated for ds (from which they were tokenized). You need to preserve that until you've done analyzing the tokens. Dereferencing those token pointers after freeing the space into which they point produces UB.

As a matter of principle, one cannot predict UB (else it would not be UB). Still, the particular results you report seem an unlikely manifestation, so perhaps there is yet more going on here.

3 Comments

Thank you! Those are very good points about contents and ds. I found the code elsewhere on stackoverflow and I didn't do a good job modifying it for my program, I guess. How should I go about the indices instead? I'm not entirely sure what's currently wrong with them
@Chylomicron, in the original version of your inner loop, you assign every field from b[n], without updating n in between. Your revised non-nested loop looks much better.
It looks better, but it still doesn't work for some reason :x And for some reason print statements don't work, making it difficult to debug. I'm using gcc compiler if that helps.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.