0

My problem is this:

#define MAX_LINE_LENGTH 100
#define MAX_RESOURCES_NUM 150
#define MAX_REQUESTS_NUM 150
#define MAX_REPAIRS_NUM 150
struct resource {
    long ID;
    char* name;
    long qty;
    sem_t freeResources;
};
struct request {
    long licensenum;
    long time;
    long repcount;
    long *repIDarr;
};
struct repair {
    long ID;
    char* name;
    long hours;
    long rescount;
    long *resIDarr;
};
struct resource* resarr[MAX_RESOURCES_NUM];
struct request* reqarr[MAX_REQUESTS_NUM];
struct repair* reparr[MAX_REPAIRS_NUM];
int timevar, threadcount;
//Func declarations
int main(int argc, char* argv[]){
    FILE *resfile, *reqfile, *repfile;
    char line[MAX_LINE_LENGTH], *refer;
    int i = 0;
    timevar = 0;
    threadcount=0;
    if(argc < 4) { 
        printf("Not enough arguments!");
        return 0;
    }
    if((resfile = fopen(argv[1], "r")) == NULL || (repfile = fopen(argv[2], "r")) == NULL || (reqfile = fopen(argv[3], "r")) == NULL){
        perror("Open file failed"); 
        return 0;
    }

    //Initialize resources
    while(fgets(line, 100, resfile) != NULL){
        char* token;
        refer = line;
        token = strsep(&refer, "\t");
        long ID = atoi(token);
        char* name = strsep(&refer, "\t");
        token = strsep(&refer, "\t");
        if(token = NULL){
            printf("X\n");
            return 0;
        }
        long qty = atol(token);
    }
    fclose(resfile);
    fclose(reqfile);
    fclose(repfile);
    return 0;
}

This is only part of the code, that is enough to reproduce the problem with.

I have a txt file called resources.txt:

13  car lift    8
17  front alignment 2
03  headlights adjust   2
10  oil drain   4
23  computerized check  2
35  pneumatic drive 4
40  ceiling winch   2
99  John Smith  1
29  air compressor  1
66  flats tub   1
88  paint gun   1

I separate any line with strsep() by TAB.

The problem is that I get a Segmentation fault as soon as I run the program.

When I delete this line:

long qty = atol(token);

at the end of main, I don't get the error.

I can't find what causes it.

The strangest thing is that when I run the program on my personal computer it runs fine (on CentOS) but when I run it in my college computers (with the same CentOS) it shows me the error.

Any ideas?

alk
  • 69,737
  • 10
  • 105
  • 255
Alon Barenboim
  • 428
  • 3
  • 11
  • @user3121023 thank you for your comment. Why my current implementation is wrong ? Your implementation looks a bit problematic for my specific program as I have to read few more files the same way and those other files have longer lines with much more TAB's.. – Alon Barenboim Jan 05 '20 at 14:09
  • You might like to have a closer look into [`strsep()`'s documentation](http://man7.org/linux/man-pages/man3/strsep.3.html). – alk Jan 05 '20 at 14:17
  • [Don't use `atol`](https://stackoverflow.com/q/17710018/995714) – phuclv Jan 05 '20 at 16:08
  • My bet goes on atol() crashing the program because token is NULL. – alk Jan 05 '20 at 17:35
  • @AnttiHaapala: Why, won't it be run? – alk Jan 05 '20 at 17:39

2 Answers2

3
  if(token = NULL){

does not what you expect.

Change it to be

  if (token == NULL) {

And to not step into this "stupid" ;) trap again, you might like to consider using "Yoda-Conditions" from now on, that is putting the "constant" to the left, like so:

  if (NULL == token) {

Because if you would have done

  if (NULL = token) {

the compiler would have complained massively.

And, BTW, if you'd have traced the code using a single-step debugger, you mostly likely would have notice this bug very soon.

alk
  • 69,737
  • 10
  • 105
  • 255
  • 1
    Re "*does not what you expect*", And your compiler should have warned you about that. Make sure to enable and head your compiler's warnings! (For `gcc`, I use `gcc -Wall -Wextra -pedantic`) – ikegami Jan 05 '20 at 13:03
  • Thank you for your comment. I wrote the if condition just to check whether it will stop the compilor from showing Segmentation Fault, but it didn't. It's true that I wrote it wrong, but the main problem is still here.. – Alon Barenboim Jan 05 '20 at 14:05
  • @AlonBarenboim: I strongly recommend that you teach yourself how to use a source code debugger. – alk Jan 05 '20 at 14:19
  • @alk I tried, it doesn't show anything wrong. It's so weired, because it sometimes run just fine, and sometimes doesn't. We use CentOS 7 as our primary OS in college, and I use the same CentOS copy the college PC's hosting, so it should be the same, but if I try to run it in their computer I get Segmentation Fault, and in my PC it works just fine! I even used Valgrind to look for memory problems, but no, 0 errors and 0 warnings, all malloc's were free'd, everything is okay. I'm so confused, it doesn't make sense at all. – Alon Barenboim Jan 05 '20 at 17:28
  • @AlonBarenboim: Did you reread the documentation to strsep() closely? – alk Jan 05 '20 at 17:36
  • @alk I did once more. Three times till now. Sorry that I cannot notice what you are pointing about, but may you tell me what should I look at exactly ? I do noticed a few things: 1. when strsep can't find the delimiter, it will NULL the stringp (first arg), so it may null the last word of the line before strtol can handle it. That's one. 2. I see that strsep is newer than C89/C90, what can cause problems, but if so, why it's working some times? Thank you for your comment. – Alon Barenboim Jan 05 '20 at 18:00
0

So I pigured out what caused the problem. I found out that fgets didn't return EOF when I thought it would, because it catches '\n' before EOF, so I ended up handling a string '\n' like it was a true line of information what caused strtol get a NULL pointer, which caused a Segmentation Fault error. The fix was relatively easy; I had to prevent from this while loop:

while(fgets(line, 100, resfile) != NULL){
    char* token;
    refer = line;
    token = strsep(&refer, "\t");
    long ID = atoi(token);
    char* name = strsep(&refer, "\t");
    token = strsep(&refer, "\t");
    if(token = NULL){
        printf("X\n");
        return 0;
    }
    long qty = atol(token);
}

to iterate when 'line' points at a string that has only '\n', so I added a condition to the while loop:

while(fgets(line, 100, resfile) != NULL && strlen(line) > 1)

And now it's perfect.

Alon Barenboim
  • 428
  • 3
  • 11