Segmentation fault with arrays

Question

I am having problems with segmentation fault. When I run this particular code on a windows machine with devc++ or codeblocks it runs how I intended. The problem occurs when I try and run on a linux machine. I was told that it was most likely due to pointers trying to access locations they are not allowed but i cannot figure out why.

When I run the program without the "empinfo.txt" file it will start and allow me to do any of the menu options except for "1-Add", so the problem seems to have something to do with the way I am using arrays with my structure.

I included all of the code but the only problems functions(i think) are initialize and add. Any help on what is the correct way to use an array of chars in this case would be highly appreciated.

Here is sample input that would be in empinfo.txt file

12   JackSprat   2     1    65000
13   HumptyDumpty  5   3    30000
17   BoPeep  2       3      30000
20   BoyBlue    3    2      58000
0

-

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

FILE *empinfo=NULL;

struct employeeData {
    int EMP_ID;
    char name[20];
    int dept;
    int rank;
    double salary;
    struct employeeData *next;
};

struct employeeData *head = NULL;

void initializeList();
void add(int ID, char name[0], int dept, int rank, double newSalary);
void deleteEmp(int ID);
void modify(int ID, double NewSalary);
void query(int rank);
void print();

int main(){

    int choice=1, ID=0, rank=0, dept=0;
    double newSalary=0;
    char name[20];
    initializeList();

    while (choice != 0) {
        printf("1-Add 2-Delete 3-Modify 4-Query 5-Print 0-Exit\n");
        fflush(stdin);
        scanf("%d", &choice);

        if (choice == 1) {
            printf("Enter new employee info. Format: \"ID name dept rank salary\"\n");
            if((scanf("%d %s %d %d %lf", &ID, name, &dept, &rank, &newSalary))==5)
                add(ID, name, dept, rank, newSalary);
            else{
                printf("invalid format entered\n\n");
                continue;
            }
        }
        else if (choice == 2){
            printf("Enter employee ID to delete: ");
            if(scanf("%d", &ID)==1){
                deleteEmp(ID);}
            else{
                printf("Employee ID must be an integer\n");
                continue;
            }
        }
        else if (choice == 3){
            printf("Enter employee ID to modify: ");
            if(scanf("%d", &ID)==1){
                printf("Enter new salary amount: ");
                if(scanf("%lf", &newSalary)==1)
                    modify(ID,newSalary);
                else
                    printf("Salary must be a number\n");
            }
            else{
                printf("Employee ID must be an integer\n");
                continue;
            }
        }
        else if (choice == 4){
            printf("Enter the rank you wish to query: ");
            scanf("%d", &rank);
            query(rank);
        }
        else if (choice == 5){
            print();
        }
    }
    printf("Goodbye...\n");
    head=NULL;
    free(head);
    return 0;
}

void initializeList(){


    empinfo=fopen("empinfo.txt", "r");
    head = (struct employeeData *)malloc(sizeof(struct employeeData));
    head->next = NULL;
    if (empinfo==NULL){
        printf("empinfo.txt not found, file not opened.\n");
        head=NULL;
        free(head);
        free(empinfo);
        return;
    }

    struct employeeData *tempPtr = head;


    while (tempPtr->EMP_ID != 0){
        fscanf(empinfo, "%d %s %d %d %lf", &tempPtr->EMP_ID, tempPtr->name, &tempPtr->dept, &tempPtr->rank, &tempPtr->salary);
        if (tempPtr->EMP_ID == 0){
            break;
            }
        tempPtr->next = (struct employeeData *)malloc(sizeof(struct employeeData));
        tempPtr=tempPtr->next;
    }
    tempPtr=head;
    while(tempPtr->next->EMP_ID!=0){
        tempPtr=tempPtr->next;
    }
    empinfo=NULL;
    free(empinfo);
    tempPtr->next=NULL;
    fclose(empinfo);
    tempPtr=NULL;
    free(tempPtr);
}

void add(int ID, char name[], int dept, int rank, double newSalary){
    struct employeeData *tempPtr = head;
    while ((tempPtr->next!=NULL) && (tempPtr->next->EMP_ID < ID)) {
        tempPtr=tempPtr->next;
    }
    struct employeeData *newNode = (struct employeeData * )malloc(sizeof(struct employeeData));
    newNode->EMP_ID = ID;
    strcpy(newNode->name,name);
    newNode->dept = dept;
    newNode->rank = rank;
    newNode->salary = newSalary;
    newNode->next=NULL;
    if (tempPtr==head) {
        if(ID>tempPtr->EMP_ID){
            newNode->next = tempPtr->next;
            tempPtr->next = newNode;
        }
        else {
            newNode->next=tempPtr;
            head=newNode;
        }
    }
    else if (tempPtr->next == NULL){
        tempPtr->next=newNode;
        newNode->next=NULL;
    }
    else{
        newNode->next = tempPtr->next;
        tempPtr->next = newNode;
    }
    printf("Employee #%d has been added to the database.\n\n", ID);
    tempPtr=NULL;
    newNode=NULL;
    free(newNode);
    free(tempPtr);

}
void deleteEmp(int ID){
    struct employeeData *seek=head,*tempPtr = head;

    if(head!=NULL) {
        while((tempPtr->EMP_ID!=ID)){
                seek = tempPtr;
                tempPtr=tempPtr->next;
                if(tempPtr==NULL){
                    printf("Employee ID not found\n");
                    free(tempPtr);
                    seek=NULL;
                    free(seek);
                    return;
                }
        }
        if (tempPtr!=NULL){
            if(tempPtr==head){
                if(tempPtr->next==NULL){
                    printf("List cannot be empty\n");
                    free(tempPtr);
                    seek=NULL;
                    free(seek);
                    return;
                }
                head=tempPtr->next;
                free(tempPtr);
            }
            else if (tempPtr->next==NULL){
                free(tempPtr);
                seek->next = NULL;
            }
            else{
                seek->next=tempPtr->next;
                free(tempPtr);
            }
        }
    }
    printf("Employee #%d has been deleted\n", ID);
    tempPtr=NULL;
    free(tempPtr);
    seek=NULL;
    free(seek);
}

void modify(int ID, double NewSalary){
    struct employeeData *tempPtr = head;

    while (tempPtr!=NULL&&tempPtr->EMP_ID!=ID){
        tempPtr=tempPtr->next;
    }
    if(tempPtr==NULL){
        printf("Employee ID not found\n");
        free(tempPtr);
        return;
    }
    tempPtr->salary=NewSalary;
    printf("Employee salary updated.\n\n");
    tempPtr=NULL;
    free(tempPtr);
}

void query(int rank){
    struct employeeData *tempPtr = head;

     while (tempPtr!=NULL){
        if(tempPtr->rank == rank){
            printf("%s\n", tempPtr->name);
            tempPtr=tempPtr->next;
        }
        else
            tempPtr=tempPtr->next;
     }
     tempPtr=NULL;
     free(tempPtr);
}

void print(){
    struct employeeData *tempPtr = head;

    while (tempPtr!=NULL){
        printf("%d %s %d %d %.0lf\n", tempPtr->EMP_ID, tempPtr->name, tempPtr->dept, tempPtr->rank, tempPtr->salary);
        tempPtr=tempPtr->next;
    }
    free(tempPtr);

The continues are unnecessary; the code in the if block before each continue ends up going automatically to the same place as the code in the else block with the continue. However, that has nothing to do with segmentation faults. — Jonathan Leffler
– Jonathan Leffler, Commented Sep 24, 2014 at 16:26
In initializeList(), it would be simpler to defer the memory allocation until after you've checked that you've opened the file successfully; there'd be less clean up to do on the error path. — Jonathan Leffler
– Jonathan Leffler, Commented Sep 24, 2014 at 16:28
Ok I did that and it still segmentation faults, so it is accessing the file i assume. — Daniel Kinney
– Daniel Kinney, Commented Sep 24, 2014 at 16:33
Why are you calling free(NULL) literally everywhere? It does literally nothing. — Bill Lynch
– Bill Lynch, Commented Sep 24, 2014 at 16:35
I've only scanned the code quickly and haven't worked out what your problem is. I think you pre-allocate the node in the list before you know whether you'll need it, which is probably not a good strategy. However, I've not understood it all. My prior suggestions were primarily cosmetic - one of the reasons they were comments, not in an answer. — Jonathan Leffler
– Jonathan Leffler, Commented Sep 24, 2014 at 16:35

megahallon · Accepted Answer · 2014-09-24 16:36:58Z

2

Use a debugger. If you are on Linux you probably have access to gdb. Here is a session with your example and the provided test file:

$ gdb test
...
(gdb) run
...
Program received signal SIGSEGV, Segmentation fault.
0x0000000000400bb2 in initializeList () at test.c:111
111     while(tempPtr->next->EMP_ID!=0){
Missing separate debuginfos, use: debuginfo-install glibc-2.18-14.fc20.x86_64
(gdb) p tempPtr
$1 = (struct employeeData *) 0x603250
(gdb) p tempPtr->next
$2 = (struct employeeData *) 0x0

We can see that we are trying to dereference a NULL pointer on line 111. It should be easy to locate the problem with this information.

This is also sure to cause errors:

empinfo=fopen(...)
...
empinfo=NULL;
free(empinfo);
fclose(empinfo);

You should not free the handle you got from fopen and you should not set it to NULL, let fclose handle this.

answered Sep 24, 2014 at 16:36

megahallon

1443 bronze badges

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

1 Comment

Daniel Kinney Over a year ago

Thank's for the help I'll try to figure it out from this info. I wasn't aware i could use the debugger while connecting to Linux server form windows machine. Thanks

Community · Accepted Answer · 2017-05-23 11:49:36Z

Welcome to the world of undefined behaviour! Beside a lot of other things, UB happens if you do this:

The value of the object allocated by the malloc function is used (7.20.3.3).

Find out more in the C99 standard, specifically take a look at Annex J. This is not a fun read, but essential if you want to use the C language in any other context than playing around.

So what happened to you is that on the Windows platform the memory returned by malloc() was obviously initialized to non-zero, including tempPtr->EMP_ID. That's why it worked there, but since it still is undefined behaviour, it is still wrong! On Linux the memory was obviously set to all-0 leading to the while() loop in initializeList() being skipped, resulting in a dereference of tempPtr->next which was set to NULL just a few lines before...

Since there're a lot of things wrong in your code, here're a few tips:

Use calloc() instead of malloc(). This initializes the memory to all-0
Do not cast the result of malloc (did this by myself for long time, but got convinced by the linked thread)
Check the return result of all your function calls, specifically memory allocations!
Set all pointer members of allocated structs manually to NULL
Initialize all variables before you use them! This may include memset()ing arrays
Read the documentation of fscanf(). Never used it by myself, but your use it not secure (buffer overflows)
Always read the documentation of system and library functions, especially the part where it comes to error handling. Currently, you do no error handling at all.
Ensure every use of free() makes sense. Just putting it everywhere where you think it may be wise is not a good coding practice :). Using it for file descriptors is just wrong.
Seriously consider another language. It's sad to see so many lines of code for such a task. C has its strengths, your case does not play them at all.

Collectives™ on Stack Overflow

Segmentation fault with arrays

2 Answers 2

1 Comment

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related