Segmentation fault core dumped in C while trying to create memory for a Struct

Question

I'm getting a segmentation fault when trying to run the following code:

C File

#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
#include <string.h>
#include "myData.h"

struct structPointer *sP;
struct structPointer *sSP;
int recordNumber = 0;
int numberOfAccesses = 0;

int main(void) {
    sP = (struct structPointer *) malloc(5 * sizeof(struct structPointer));
    sSP = sP;

    memcpy(sP->name,"Adam Baum",50);
    memcpy(sP->fireNumber,"N1234",10);
    memcpy(sP->street,"Top Secret",25);
    memcpy(sP->city,"Manhattan",25);
    memcpy(sP->state,"New York",25);
    sP++;
    recordNumber++;
    memcpy(sP->name,"Adam Zapel",50);
    memcpy(sP->fireNumber,"S4321",10);
    memcpy(sP->street,"Throat",25);
    memcpy(sP->city,"Manhattan",25);
    memcpy(sP->state,"New York",25);
    sP++;
    recordNumber++;
    memcpy(sP->name,"Al Bino",50);
    memcpy(sP->fireNumber,"W1234",10);
    memcpy(sP->street,"White",25);
    memcpy(sP->city,"Anchorage",25);
    memcpy(sP->state,"Alaska",25);
    sP++;
    recordNumber++;
    memcpy(sP->name,"Anne Teak",50);
    memcpy(sP->fireNumber,"E4321",10);
    memcpy(sP->street,"Oak",25);
    memcpy(sP->city,"Woodville",25);
    memcpy(sP->state,"Wisconsin",25);
    sP++;
    recordNumber++;
    memcpy(sP->name,"Barb Dwyer",50);
    memcpy(sP->fireNumber,"N1234",10);
    memcpy(sP->street,"Keepout",25);
    memcpy(sP->city,"Kilgore",25);
    memcpy(sP->state,"Texas",25);
    recordNumber++;

    while (1){
        int sel;
        printf("MENU\n");
        printf("=====\n");
        printf("1. Print All Records\n");
        printf("2. Print Number of Records\n");
        printf("3. Print Size of Database\n");
        printf("4. Add Record\n");
        printf("5. Delete Record\n");
        printf("6. Print Number of Accesses to Database\n");
        printf("7. Exit\n");
        printf("=====\n");
        printf("Enter selection: ");
        sel = getchar();
        sel = sel-48;
        switch(sel){
            case 1:
                numberOfAccesses++;
                printAllRecords(sP);
                break;
            case 2:
                numberOfAccesses++;
                fprintf(stderr,"There are a Total of %d records\n", recordNumber);
                break;
            case 3:
                numberOfAccesses++;
                printSizeOfDatabase(sP);
                break;
            case 4:
                numberOfAccesses++;
                sP = sSP;
                addRecord(sP);
                break;
            case 5:
                numberOfAccesses++;
                deleteRecord(sP);
                break;
            case 6:
                numberOfAccesses++;
                fprintf(stderr,"The total number of Accesses is %d\n", numberOfAccesses);
                break;
            case 7:
                exit(0);
            case -38:
                printf("Code is reading in LineFeed and displaying Menu again\n");
                break;
            default:
                printf("Error: Input was not a valid selection.\n");
                break;
        }
    }
return 0;
}

int printAllRecords(struct structPointer *addresses){
    int i;
    printf("All Records: ");
    for(i=1;i<recordNumber;i++){
        printf("Address: %d\n", i);
        fprintf(stderr, "Name = \%s\n", addresses-> name);
        fprintf(stderr, "Fire Number = \%s\n", addresses-> fireNumber);
        fprintf(stderr, "Street = \%s\n", addresses-> street);
        fprintf(stderr, "City = \%s\n\n", addresses-> city);
        fprintf(stderr, "State = \%s\n\n", addresses-> state);
    }
    return 1;
}

int printSizeOfDatabase(struct structPointer *addressesAgain) {
    int size = 0;
    int i;
        for (i=1;i<=recordNumber;i++) {
        size += sizeof(addressesAgain->name);
        size += sizeof(addressesAgain->fireNumber); 
        size += sizeof(addressesAgain->street);
        size += sizeof(addressesAgain->city);
        size += sizeof(addressesAgain->state);
        addressesAgain++;
        }
    fprintf(stderr, "The size of the database is %d bytes.\n", size);
    return size;
}

int addRecord(struct structPointer *addressesAgainTimes2){
    char entryName;
    char entryFireNumber;
    char entryStreet;
    char entryCity;
    char entryState;
    recordNumber++;
    struct structPointer *theStruct;
    theStruct = (struct structPointer *) malloc ((recordNumber+1) * sizeof(struct structPointer));
    addressesAgainTimes2 = sSP;
    int i;
    for (i=1;i<recordNumber;i++){
        memcpy(theStruct->name,addressesAgainTimes2->name,50);
        memcpy(theStruct->fireNumber,addressesAgainTimes2->fireNumber,10);
        memcpy(theStruct->street,addressesAgainTimes2->street,25);
        memcpy(theStruct->city,addressesAgainTimes2->city,25);
        memcpy(theStruct->state,addressesAgainTimes2->state,25);
        if(i==recordNumber-1){
            theStruct++;}
        else{
            theStruct++;
            addressesAgainTimes2++;}
    }
    printf("Enter the Name of the New Record: \n");
    scanf("%s",&entryName);
    memcpy(theStruct->name,&entryName,50);
    printf("Enter the Fire Number of the New Record: \n");
    scanf("%s",&entryFireNumber);
    memcpy(theStruct->fireNumber,&entryFireNumber,10);
    printf("Enter the Street of the New Record: \n");
    scanf("%s",&entryStreet);
    memcpy(theStruct->street,&entryStreet,25);
    printf("Enter the City of the New Record: \n");
    scanf("%s",&entryCity);
    memcpy(theStruct->city,&entryCity,25);
    printf("Enter the State of the New Record: \n");
    scanf("%s",&entryState);
    memcpy(theStruct->state,&entryState,25);
    addressesAgainTimes2=theStruct;
    printf("Record has been added.");
    return 0;
}

int deleteRecord(struct structPointer *addressesAgainTimes3){
    struct structPointer *anotherStruct;
    anotherStruct = (struct structPointer *) malloc ((recordNumber+1) * sizeof(struct structPointer));
    int i;
    for(i=0;i<5;i++){
        memcpy(anotherStruct->name,addressesAgainTimes3->name,50);
        memcpy(anotherStruct->fireNumber,addressesAgainTimes3->fireNumber,10);
        memcpy(anotherStruct->street,addressesAgainTimes3->street,25);
        memcpy(anotherStruct->city,addressesAgainTimes3->city,25);
        memcpy(anotherStruct->state,addressesAgainTimes3->state,25);
        addressesAgainTimes3++;
    }
    addressesAgainTimes3=anotherStruct;
    recordNumber--;
    printf("Record has been deleted.");
    return 0;
}

Header File:

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

struct structPointer{
    char *name;
    char *fireNumber;
    char *street;
    char *city;
    char *state;
};

This program is supposed to create a chunk of memory where I can store a struct and display a menu where I can add/remove entries from that struct.

"memcpy(sP->name,"Adam Baum",50);" - name is uninitialized here, same as all the other struct members. — Andrew C, Oct 10 '14 at 22:13
In addition to dereferencing uninitialized pointers, you're also copying out of bounds of the strings. `memcpy(sP->name, "Adam Baum", 50)` will try to copy 50 bytes from a 10-byte string. — Barmar, Oct 10 '14 at 22:17
PS, don't cast the result of malloc(): http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — Barmar, Oct 10 '14 at 22:18

score 2 · Answer 1 · answered Oct 10 '14 at 22:17

2

If you don't want to have to initialize the struct members or manage their memory separately, then they should be arrays instead of pointers:

struct structPointer{
    char name[50];
    char fireNumber[10];
    char street[25];
    char city[25];
    char state[25];
};

answered Oct 10 '14 at 22:17

John Bollinger

160,171
8
81
157

Watch out, too, that you reserve space for the strings' trailing null byte. The sizes above correspond to your `memcpy()` lengths, but if used as strings then those arrays accommodate one fewer char than their declared lengths. – John Bollinger Oct 10 '14 at 22:21
Very helpful, thank you! It's working now until I try to add a record. Once I enter the state it seg faults again. Any ideas? – Specerion Oct 10 '14 at 22:27
You reserve only enough space for five records. If you try to add a sixth then you will end up writing in memory that is not part of the block you reserved. Count yourself lucky that that yields a segfault. To address the problem you could attempt to `realloc()` at need to get a larger block, or you could turn your array of structs into a linked list, allocating (and deallocating) on a per-struct basis. – John Bollinger Oct 13 '14 at 16:28

Ron Pinkas · Accepted Answer · 2014-10-10T22:24:07.223

1

Your structPointer member fields are POINTERS to strings, but they are NEVER allocated any memory. You then use memcpy() in effect copying to an UNDEFINED memory.

Changing your structure as follows will resolve that specific issue:

struct structPointer{
    char name[SOME_APPROPRIATE_NAME_LENGTH];
    char fireNumber[SOME_APPROPRIATE_NUMBER_LENGTH];
    char street[SOME_APPROPRIATE_STREET_LENGTH];
    char city[SOME_APPROPRIATE_CITY_LENGTH];
    char state[2];
};

edited Oct 10 '14 at 22:24

answered Oct 10 '14 at 22:16

Ron Pinkas

357
2
10

This solved the problem, thank you! Any idea why it would be segfaulting after trying to add a record? (Once I hit enter after typing in a state) – Specerion Oct 10 '14 at 22:33
I just noticed you used States NAMES not Abbreviations, so how much space have you allocated to the State in your structure? – Ron Pinkas Oct 10 '14 at 22:43
Same exact problem in addRecord, where all your entry... vars are not allocated any space, instead they should be arrays of appropriate space. But the code of addRecord could be greatly simplified by using realloc() instead of allocating new structure and copying to it, and also using scanf() in place, directly to the struct members and avoiding also that memcpy operation. – Ron Pinkas Oct 11 '14 at 01:58

score 0 · Answer 3 · answered Oct 10 '14 at 22:17

When you initialise it, you allocate room for five times a struct, but within the struct, the char * (like name) are unallocated pointers. You should allocate room for them (and free them when clearing). Either with strdup, or a malloc/memcpy. If they are fixed size, declare them as fixed size arrays, and then they're OK as well.

score 0 · Answer 4 · answered Oct 10 '14 at 22:23

0

Have your struct members as arrays or separately allocate memory for each of them before you copy.

answered Oct 10 '14 at 22:23

Madhu

21
3

Segmentation fault core dumped in C while trying to create memory for a Struct

4 Answers4