segmentation fault problem

elenizi · 10-07-2012, 02:57 PM

hi i want to make a parser that reads from a file and saves the tokens into a struct i am using this code but i keep getting segmentation fault

Code:

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

typedef struct{
  int label;
  int id;
  int r1;
  int r2;
  int r3;
}instruction;

int main(int argc,char *argv[])
{
    FILE *input;
    char str[1000];   
    char delim[] = " ";
    char *result;

 input = fopen(argv[1], "r"); // error check this!
 int i=0;
    instruction *instr = (instruction*)(malloc(100*sizeof(instruction)));
    while  (fgets(str,1000,input)!=NULL){
  
        result = strtok(str, delim);
        instr[i].label = atoi(result);

        result = strtok(NULL, delim);
        instr[i].id = atoi(result);
 
	result = strtok(str, delim);
        instr[i].r1 = atoi(result);  
         
	result = strtok(NULL, delim);        
        instr[i].r2 = atoi(result); 

	result = strtok(str,delim);
        instr[i].r3 = atoi(result); 
 
       i++;
    }
    fclose(input);
    return 0;
}

JohnGraham · 10-07-2012, 04:29 PM

Quote:

Originally Posted by elenizi

Code:

 input = fopen(argv[1], "r"); // error check this!

Always always always always check your return values - do that to your code right now. Anything else is wasting everyone's time (most importantly yours). Always always always always. Always.

Aquarius_Girl · 10-08-2012, 12:56 AM

Does the file which you are trying to open with fopen exists
where it should exist?

elenizi · 10-08-2012, 02:01 AM

i have checked the file and it opens fine...
the file is saved in the same folder as the exe of the code and i have created so it is also not empty...
when i remove the save option in an array i mean the instr line it works perfectly and when i replace it with a printf to see the content of the token result it is also ok... for some reason my problem appears when i try to save the content of resume in a struct...

Code:

result = strtok(str, delim);
instr[i].label = atoi(result);

bigearsbilly · 10-08-2012, 05:19 AM

cut and paste error, red should be NULL

Quote:

Originally Posted by elenizi

hi i want to make a parser that reads from a file and saves the tokens into a struct i am using this code but i keep getting segmentation fault

Code:

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

typedef struct{
  int label;
  int id;
  int r1;
  int r2;
  int r3;
}instruction;

int main(int argc,char *argv[])
{
    FILE *input;
    char str[1000];   
    char delim[] = " ";
    char *result;

 input = fopen(argv[1], "r"); // error check this!
 int i=0;
    instruction *instr = (instruction*)(malloc(100*sizeof(instruction)));
    while  (fgets(str,1000,input)!=NULL){
  
        result = strtok(str, delim);
        instr[i].label = atoi(result);

        result = strtok(NULL, delim);
        instr[i].id = atoi(result);
 
	result = strtok(str, delim);
        instr[i].r1 = atoi(result);  
         
	result = strtok(NULL, delim);        
        instr[i].r2 = atoi(result); 

	result = strtok(str,delim);
        instr[i].r3 = atoi(result); 
 
       i++;
    }
    fclose(input);
    return 0;
}

you may as well get rid of the malloc and do this:

Code:

    static instruction instr[100];

avoid malloc it just makes more work (you never checked the malloc return bad habit too)

elenizi · 10-08-2012, 11:16 AM

i haven't used c for a very long time so i am very rusty... what do you mean cut and paste error? which value should i change?

johnsfine · 10-08-2012, 11:49 AM

Quote:

Originally Posted by bigearsbilly

cut and paste error

That was a plausible guess at the editing operation (during construction of the program) that caused the programmer to make the mistake. But it isn't directly relevant to fixing the error and it seems to have confused the OP.

Quote:

red should be NULL

Hard to be much clearer than that.

Quote:

Originally Posted by elenizi

i haven't used c for a very long time so i am very rusty.

Did you write the code you posted? Or just copy it from somewhere?

Quote:

which value should i change?

The places marked in red in the quote of your code.

If you had written the code it is hard to imagine why you would need any more than the red marks highlighting the location of the mistake. But bigearsbilly also told you what to change the wrong items to.

Since you didn't ask what he meant by "red" I assume your browser lets you see which spots were marked in red.

elenizi · 10-08-2012, 11:59 AM

i did write the code after a lot of reading about strtok and after experimenting for many hours.. that's why i was confused that only when i tried to save the tokens i was cutting my string into i got the segmentation fault and posted my code so maybe someone with a clearer mind and more experience could help me solve this... i could not see the red markings that's why i was confused but i will assume it is the str value which is bolded? i will try all of your advices and let you know if the problem was solved! thank you very much everyone!!! =)

elenizi · 10-08-2012, 02:01 PM

i have made all the changes you said but the segmentation fault still appears =/ i am using cygwin for this project could that be the reason it is not working?

johnsfine · 10-08-2012, 02:40 PM

Are you sure the input file is short enough for your allocation of 100 instructions?
It is much safer to have code to stop at the allocated size, even if end of file has not been reached.

Are you sure you understood which things were red and changed them? It is usually best to post the current code when you have made changes but the program still doesn't work.

Quote:

Originally Posted by elenizi

i am using cygwin for this project could that be the reason it is not working?

I don't think that is likely enough to be worth worrying about at all. Most likely the problem is that the loop doesn't stop when the index variable (i) gets too big, either because the input file is too long or because of some bug I can't see.

bigearsbilly · 10-08-2012, 05:11 PM

Sorry if i wasn't clear.
Subsequent calls to strtok should have a NULL pointer where your code erroneously passes
str in again.
I only tested with a single line file.
You should include your input file (or part of) if you want help.

If you have a class or struct it's a good idea to have
a print method for debugging.

You should get used to a debugger like gdb if you want to use C.

Avoid malloc it's not worth the extra work for simple tools and utilities (in my opinion) that run for half a second, I rarely use it myself allocating a few Kb won't even dent modern systems - get your program working first then fine-tune it if needs be.

bigearsbilly · 10-09-2012, 01:52 AM

Here is a slightly cleaned up version.
You weren't checking the fopen and not checking
that argv[1] contained anything. A usage is always a good idea, if you go back
3 months later you wonder what the hell it's for!

There are still problems, the strtok returns aren't checked before
you assign to the the struct - but this is OK if you state the assumption that
your input file is correct, which is a valid technique, I use it for efficiency
quite often. You could scanf also which does minimal parsing.

Anyway I'd better get back to work, got a real time data feed to knock up ;-)

Code:

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

typedef struct{
  int label;
  int id;
  int r1;
  int r2;
  int r3;
} instruction;

static instruction array[100];

void usage(char * message)
{
    fprintf(stderr, "Oops! %s\n", message);
    exit(1);
}
int main(int argc,char *argv[])
{
    FILE *input;
    char str[1000];   
    char delim[] = " ";
    char *result;

    if(argc != 2) {
        usage("need a filename");
    }

    input = fopen(argv[1], "r"); // error check this!
    if(!input) {
        perror(argv[1]);
        return 1;
    }

    int i=0;
    while  (fgets(str,1000,input)!=NULL){
  
        result = strtok(str, delim);
        array[i].label = atoi(result);

        result = strtok(NULL, delim);
        array[i].id = atoi(result);
 
        result = strtok(NULL, delim);
        array[i].r1 = atoi(result);  
         
        result = strtok(NULL, delim);        
        array[i].r2 = atoi(result); 

        result = strtok(NULL,delim);
        array[i].r3 = atoi(result); 
 
        i++;
        if(i > ( sizeof array / sizeof (instruction))) {
            printf("memory used up\n");
            break;
        }
    }
    fclose(input);
    return 0;
}

johnsfine · 10-09-2012, 08:17 AM

Quote:

Originally Posted by bigearsbilly

Avoid malloc it's not worth the extra work for simple tools and utilities (in my opinion) that run for half a second, I rarely use it myself allocating a few Kb won't even dent modern systems - get your program working first then fine-tune it if needs be.

I don't understand your objection to malloc. I want to make sure Elenizi understands that the objection to malloc is just your opinion. (In contrast to what you said about correct use of strtok, which gave a necessary correction, not an opinion).

The removal of malloc from the revised version of the program you posted was not directly relevant to fixing any problem. Checking whether the index (i) has become too big might be critical to fixing the problem. The way you chose to code that check depended on your change removing malloc. But that check could have been coded just as well without removing malloc.

You are correct that most of the reasons one shouldn't use static allocation that way in a major project don't matter in a trivial project. But malloc is not hard to use (especially in a trivial project, where there is no real need to balance it with free). So lack of a reason to not use static does not create a reason to not use malloc.

elenizi · 10-11-2012, 02:18 AM

maybe to help you understand what i am trying to to i want to read a file that contains some kind of instructions then check if the instruction i got is included in a special instruction set and save it so i can use it to do calculations.the problem is when i do check the instruction exists in my set i have to cut the instruction into pieces and save each part in a struct... that's why i first tried to make the cutting and saving part work and then add the check to define what kind of instruction i got. i created my input file so i am sure of it's contents and that they are correct.

johnsfine · 10-11-2012, 06:43 AM

Quote:

Originally Posted by johnsfine

Are you sure you understood which things were red and changed them? It is usually best to post the current code when you have made changes but the program still doesn't work.

Or did you try the version bigearsbilly posted?

We can't guess what code you are running and try to guess what is causing whatever problem you are still seeing.