CODEWITHSUNDEEP
cinputlinked-listscanf
Dawson
Dawson

Reputation: 457

Segmentation fault when inserting values from file into linked list

I want to read values from a text file with has a string int int format like so:

testing 5 17
charlie 12 1
delta 88 4

I have a function that reads the file using fscanf, puts the input into some variables then send them to a function that inserts them as a linked list node:

void readFile(LinkedList *inList, char* file)
    {
    char* tempName;
    int tempLoc, tempNum;

    FILE* f;
    f = fopen(file, "r");
    if(f==NULL) 
        {
        printf("Error: could not open file");
        }
    else
        {
        while (fscanf(f, "%s %d %d", tempName, &tempLoc, &tempNum) != EOF)
            {
            insertFirst (inList, tempName, tempLoc, tempNum);
            }
        }   
    }

The insertFirst function:

void insertFirst(LinkedList* list, char* inName, int inLoc, int inNumMeth)
    {
    LinkedListNode* newNode;
    newNode = (LinkedListNode*)malloc(sizeof(LinkedListNode));
    newNode->className = inName;
    newNode->loc = inLoc;
    newNode->numMethods;
    newNode->next = list->head;
    list->head = newNode;
    }

When I traverse the linked list to print out the values, it gives strange symbols for the name (�t) and incorrect numbers for the ints before crashing with a segmentation fault. I'm having trouble tracking down the cause.

Upvotes: 0

Views: 328

Answers (5)

simonc
simonc

Reputation: 42185

Your fscanf call is writing to an uninitialised pointer. This invokes undefined behaviour and its a little surprising it doesn't crash.

You need to allocate storage for the string

char tempName[30];

and should also modify your fscanf call to read at most this many chars and check that all 3 of name, location, method were read

while (fscanf(f, "%29s %d %d", tempName, &tempLoc, &tempNum) == 3)

As noted by Marcus, you also need to allocate storage for className in LinkedListNode. The easiest way is to make className a 30 element char array and use strcpy

strcpy(newNode->className, inName);

or you could also leave className as a char* and dynamically allocate its memory

newNode->className = malloc(strlen(inName)+1);
/* check for newNode->className != NULL */
strcpy(newNode->className, inName);

If you do this, make sure to free className when you free the node.

Upvotes: 3

Krishna
Krishna

Reputation: 1382

Also you should store null in the last linked list as the data reading from the file is finished.

Upvotes: 0

vinod
vinod

Reputation: 132

char* tempName;

tempName is pointing to random memory.So allocate the memory for tempName.

Return Value of fscanf : On success, the function returns the number of items of the argument list successfully filled. This count can match the expected number of items or be less (even zero) due to a matching failure, a reading error, or the reach of the end-of-file.

Upvotes: 0

MOHAMED
MOHAMED

Reputation: 43558

char* tempName;

is not pointed to a memory area. and you have used in the fscanf without assigning it to a memory:

You can fix that by changing the tempname declaration:

char tempName[MAX_NAME_SIZE];

or you can keep the declaration and use "%ms" instead of "%s" in the format string specifier in the fscanf

fscanf(f, "%ms %d %d", &tempName, &tempLoc, &tempNum)

%ms will allow to fscanf to allocate memory for tempName. you have to free this memory with free() when your memory become useless

Note %ms is valid if the gcc version is > gcc 2.7

Upvotes: 1

Bathsheba
Bathsheba

Reputation: 234795

You also need to deep copy inName; all you are doing in

newNode->className = inName

is copying the pointer, not the actual string data. You can use strcpy to do that.

Upvotes: 0

Related Questions