Segfault while Reading text file into structure pointer

Question

I am writing a code to compute jedi name which is a combination of first name and last name. I have written the entire code and getting segmentation fault in the while loop.

Following is the code. It is divided into a header and a C file:

Structures.h

//void jediName(char *first_name, char *last_name, char buffer[10]);
//void jediName(struct Names Name_Param);
//void * allocate(unsigned int size);
//void * deallocate(void *, int size);

int heap_usage = 0;

struct Names{
    char *first_name;
    char *last_name;
    char *jedi_name;
};

struct Names *name;

void jediName(struct Names *Name_Param);
void * allocate(unsigned int size);
void * deallocate(void *, int size);

Program.c

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

#include "Structures.h"

int main(){

//  char fname[10];
//  char lname[10];
//  char buff[10]= "";
    char buffer1[250];
    char buffer2[250];

    FILE * fp;
/*
        printf("Enter the first & last name : \n");
        scanf("%s %s", buffer1, buffer2 );

    fp = fopen("names.txt", "a");
    fprintf(fp, "\n%s %s", buffer1, buffer2);
    fclose(fp);
*/

    fp = fopen("names.txt","r");
    name->first_name == allocate(10);
    name->last_name == allocate(10);
    name->jedi_name == allocate(10);

    while(!feof(fp)){
        fscanf(fp, "%s %s", name->first_name, name->last_name);
        jediName(name);
//      jediName(name->first_name, name->last_name, name->jedi_name);
        printf("%s %s %s", name->first_name, name->last_name);
    }
    deallocate(name->first_name, 10);
        deallocate(name->last_name, 10);
        deallocate(name->jedi_name, 10);

    fclose(fp);

    return 0;
}

/*
void jediName(char *first_name, char *last_name, char buffer[10]){
    if(strlen(first_name)<2 || strlen(last_name)<3)
        printf("Name of %s %s is too short to compute a jedi name\n", first_name, last_name);
    else{
        buffer[0] = last_name[0];
            buffer[1] = last_name[1];
            buffer[2] = last_name[2];
            buffer[3] = first_name[0];
            buffer[4] = first_name[1];
            printf("Jedi Name for %s %s is %s\n", first_name, last_name, buffer);
    }

    return;
}
*/

void jediName(struct Names *Name_Param){
        if(strlen(Name_Param->first_name)<2 || strlen(Name_Param->last_name)<3)
                printf("Name of %s %s is too short to compute a jedi name\n", Name_Param->first_name, Name_Param->last_name);
        else{
                Name_Param->jedi_name[0] = Name_Param->last_name[0];
                Name_Param->jedi_name[1] = Name_Param->last_name[1];
                Name_Param->jedi_name[2] = Name_Param->last_name[2];
                Name_Param->jedi_name[3] = Name_Param->first_name[0];
                Name_Param->jedi_name[4] = Name_Param->first_name[1];
                printf("Jedi Name for %s %s is %s\n", Name_Param->first_name, Name_Param->last_name, Name_Param->jedi_name);
        }

        return;
}

void * allocate(unsigned int size){
    heap_usage = heap_usage + size;
    printf("The current heap size after heap allocation is %d\n", heap_usage);
    void *heapMem = malloc(size);
    if(heapMem == NULL)
        printf("Pointer is NULL\n");
    else
        printf("Pointer is not NULL\n");

    return heapMem;
}

void * deallocate(void *heapMem, int size){
    heap_usage = heap_usage - size;
    printf("The current heap size after heap deallocation is %d\n", heap_usage);
    free(heapMem);
    heapMem = NULL;
    return NULL;
}

I have to allocate and deallocate memory to the pointers using a function and keep track of the heap memory, that is the reason I have those functions — , Nov 10 '17 at 02:40
you're not allocating any space for `name`, so `name->` is undefined behavior — yano, Nov 10 '17 at 02:41
Can you please elaborate? do you mean I need to do this? name == allocate(10); cause doing the above is not solving the error — , Nov 10 '17 at 02:43

yano · Answer 1 · 2017-11-10T03:00:00.410

2

You're invoking undefined behavior when you try to dereference name because you're not allocating any space for it. You have a pointer in name that points to nowhere (I believe it's actually initialized to 0 since it's static). In main you must do something like name = malloc(sizeof *name); or you can change the declaration to struct Names name;

On second look, name is only used in main, so just declare it there. No need for it to be declared in broader scope.

int main(void)
{
  // no point in dynamically allocating memory in this case. You don't need
  // much and you know exactly how much you need (just 1 struct)
  struct Names name;
  // but if you want to dynamically allocate it..
  // struct Names* name = malloc(sizeof *name);
  ....
  // change all your "name->" to "name." if you did not malloc
  // if you did not malloc, you must pass the address of name
  // to jediName
  // jediName(&name);

  // .. do you work

  // if you used malloc above, don't forget to free your memory
  // free(name);

  return 0;
}

Also see Why is “while ( !feof (file) )” always wrong?

edited Nov 10 '17 at 03:00

answered Nov 10 '17 at 02:46

yano

4,827
2
23
35

I have used malloc indirectly in the allocate() and also freed and set pointer to NULL in the deallocate() tried to declare name in main did not help – Nov 10 '17 at 02:59
@SagarRikame you've used it for the fields/members of `struct Name`.. you haven't used it on the struct itself! `name` needs memory too. As coded, `name` is a pointer just like `first_name`, `last_name`, and `jedi_name`. Pointers must point to already-existing memory or must have their own memory allocated before they can be dereferenced. – yano Nov 10 '17 at 03:01
name == allocate(sizeof(*name)); deallocate(name->first_name, sizeof(name->first_name)); added these 2 lines at the start and end of allocation and deallocation code still the error persists – Nov 10 '17 at 03:06
@SagarRikame use the assignment operator `=`, not the equality operator `==`. I would expect a compiler warning about that. Be sure you are allocating _before_ you do `name->`. There could be other problems, this is the only one I've looked at. – yano Nov 10 '17 at 03:10
@SagarRikame Yeah I see now you're using `==` in many places where you should be using assignment `=` – yano Nov 10 '17 at 03:11
if I use '=' then I get segfault immediately after the first allocate() statement – Nov 10 '17 at 03:12
@SagarRikame I recommend listening to any and all compiler warnings (treat them as errors). Compile with `gcc -Wall -Wextra myProg.c`, that should be plenty, and step through with a debugger. Doing an assignment with `==` is wrong. I don't see anything wrong in the `allocate` function. – yano Nov 10 '17 at 03:16
I have used the '=' and I am able to compute the jedi name for the first name in the text file, and then getting segfaut :( – Nov 10 '17 at 03:18
@SagarRikame you should also check the return values of everything,, you are with `malloc` that's good. `fopen` and `fscanf` also return values. Check their man pages to see what they return. – yano Nov 10 '17 at 03:21

score 0 · Answer 2 · answered Nov 10 '17 at 03:53

Thanks a lot for the help. THe main issues in the code were the missing memory allocation for name, the '==' and a few other extra code which was written while debugging the code.

following is the final c file.

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

#include "Structures.h"

int main(){

    char buffer1[250];
    char buffer2[250];

    FILE * fp;

        printf("Enter the first & last name : \n");
        scanf("%s %s", buffer1, buffer2 );

    fp = fopen("names.txt", "a");
    fprintf(fp, "\n%s %s", buffer1, buffer2);
    fclose(fp);


    fp = fopen("names.txt","r");

        name = allocate(10);
        name->first_name = allocate(10);
        name->last_name = allocate(10);
        name->jedi_name = allocate(10);

    while(!feof(fp)){
        fscanf(fp, "%s %s", name->first_name, name->last_name);
        jediName(name);
    }

    deallocate(name->first_name, 10);
        deallocate(name->last_name, 10);
        deallocate(name->jedi_name, 10);
        deallocate(name, 10);

    fclose(fp);

    return 0;
}

void jediName(struct Names *Name_Param){
        if(strlen(Name_Param->first_name)<2 || strlen(Name_Param->last_name)<3)
                printf("Name of %s %s is too short to compute a jedi name\n", Name_Param->first_name, Name_Param->last_name);
        else{
                Name_Param->jedi_name[0] = Name_Param->last_name[0];
                Name_Param->jedi_name[1] = Name_Param->last_name[1];
                Name_Param->jedi_name[2] = Name_Param->last_name[2];
                Name_Param->jedi_name[3] = Name_Param->first_name[0];
                Name_Param->jedi_name[4] = Name_Param->first_name[1];
                printf("Jedi Name for %s %s is %s\n", Name_Param->first_name, Name_Param->last_name, Name_Param->jedi_name);
        }

        return;
}

void * allocate(unsigned int size){
    heap_usage = heap_usage + size;
    printf("The current heap size after heap allocation is %d\n", heap_usage);
    void *heapMem = malloc(size);
    if(heapMem == NULL)
        printf("Pointer is NULL\n");
    else
        printf("Pointer is not NULL\n");

    return heapMem;
}

void * deallocate(void *heapMem, int size){
    heap_usage = heap_usage - size;
    printf("The current heap size after heap deallocation is %d\n", heap_usage);
    free(heapMem);
    heapMem = NULL;
    return NULL;
}

Segfault while Reading text file into structure pointer

2 Answers2