Why does calling a function in a different order cause a segmentation fault in C?

Question

I am working on a program in C that processes an array of character pointers. I have several functions that operate on this array, including longest_name(), unique(), and others. When I call the longest_name() function after unique() in the main() function, the program runs fine. But when I switch the order and call longest_name() before unique(), I get a segmentation fault.

I know that the pointer char *name in the longest_name() function should be allocated by malloc to avoid the segmentation fault. However, I want to understand why the program is working in one scenario but not in the other.

My suspicion is that the issue may be related to memory allocation or pointer manipulation, but I am not sure where to start looking. Can anyone explain why this is happening and how to fix it?

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


void display_name(char **stu, int indx);
void vowel_count(char **stu, int indx);
void longest_name(char **stu);
void unique(char **stu);



int main()
{
    char *name[10] = {"Aakash", "Anirudha", "Vikas", "Vinay", "Rakesh", "Thomas", "Jerry", "Alekha", "Daksh", "Peter"};
    display_name(name, 4);
    vowel_count(name,9);
    
    //longest_name(name);       //not run
    
    unique(name);
    
    longest_name(name);         //run

    return 0;
}

void unique(char **stu){
    int len = 0, found = 0;
    printf("Name whose first and last character is \'a\' :- ");
    for(int i = 0; i < 10; i++){
        len = strlen(stu[i]);
        if(stu[i][0] == 'a' && stu[i][0] == 'a'){
            found = 1;
            printf("%s\n", stu[i]);
        }
    }
    if(found == 0){
        printf("None\n");
    }
}

void longest_name(char **stu){
    int len = 0, mx = 0;
    char *name;
    printf("\n Address is %p \n",name);
    for(int i = 0; i < 10; i++){
        if(mx < strlen(stu[i])){
            mx = strlen(stu[i]);
            strcpy(name, stu[i]);
        }
    }
    printf("Longest name is \"%s\" with length %d\n", name, mx);
}

void vowel_count(char **stu, int indx){
    
    indx--;
    for(int i = 0; i < 10; i++){
        if(i == indx){
            int len = strlen(stu[i]), cnt = 0;
            char name[len];
            strcpy(name, stu[i]);
            for(int j = 0; j < len; j++){
                char c = tolower(name[j]);
                if(c == 'a' || c == 'e' || c == 'i' || c == 'o' || c == 'u')
                   cnt++;
            }
            printf("Number of vowels in \"%s\" are :- %d\n", name, cnt);
            break;
        }
    }
    
}

void display_name(char **stu, int indx){
    indx--;
    for(int i = 0; i < 10; i++){
        if(i == indx)
            printf("%s\n",stu[i]);
    }
}

I tried running the program on a different machine as I thought the issue might be related to the compiler. However, the behavior was the same on the other machine as well.

Answer 1

As mentioned in the comments, your variable name in your longest_name function is uninitialised.

You have declared it like this:

char *name;

and have not made it point anywhere.

Many compilers will manually initialise the pointer to zero, such as having written:

char *name = NULL;

... but this is not guaranteed, so it is good practice to always initialise your pointers to NULL if you do not wish to make them point anywhere yet. Nonetheless, it is important to note that initialising to NULL just means the pointer definitely does not point anywhere.

In your case, it looks like your name pointer might have been initialised to some random value - such as whetever was at that location on the stack previously. If the latter is the case, it would explain why your program works sometimes, and at other times it does not. As mentioned by a comment, the order with which you call the functions will determine what is on the stack (beneath the stack pointer) by the time the second function is called. Thus, it is perfectly plausible that when you call your unique function first and THEN your longest_name function, by sheer luck your name variable in your longest_name function is initialised to some "valid" memory location, which means you are able to write data to it.

What is happening is described as undefined behaviour. Essentially, this means that your program can sometimes perform as you expected, and sometimes do something completely different. Undefined behaviour happens when something in the program has not been written correctly, but that does not necessarily always make the program crash instantly. However, if you write a program correctly, you can avoid UB (undefined behaviour) completely.

Thus, you should never do something like:

char *whatever = "It was a sunny day.";
char *str;
strcpy(str, whatever);

... because you have not made the pointer str point anywhere valid, and you cannot copy data to a memory location that does not exist or one that cannot be accessed by your program.

In your case, your longest_name function should allocate memory and make the name pointer point to this allocated memory, before copying anything to it, such as:

name = malloc((strlen(stu[i]) + 1)*sizeof(char));
strcpy(name, stu[i]);

... remembering to free the memory after using it.

Remember, a string stored as a char* always needs to include an extra byte for the null terminator character '\0' or simply 0 in ASCII. This is where you have gone wrong in your vowel_count function, your declaration of name should be:

char name[len + 1];

Note also that by declaring name like that you are declaring a variable-length array (VLA), which can be tricky. If you need memory with a dynamic size (determined at runtime) it is usually better to use dynamic memory allocation (using malloc, and free for deallocation).

Furthermore, in your longest_name function, you don't need to allocate any extra memory, all you need is to make your name pointer point to the longest string, and print that out, such as:

void longest_name(char **stu){
    size_t len = 0, mx = 0; // strlen returns a number of type size_t
    char *name = NULL; // initialise to NULL since not pointing anywhere
    printf("\n Address is %p\n", name); // this will either be zero or undefined
    for(unsigned int i = 0; i < 10; i++){ // use unsigned as you start from zero
        if(mx < (len = strlen(stu[i]))){ // assign len to only call strlen once
            mx = len;
            name = stu[i]; // simply make name point to the longest string
        }
    }
    printf("Longest name is \"%s\" with length %zu\n", name, mx);
}

In conclusion, your program runs sometimes and at other times it crashes, becuase your name variable sometimes ends up pointing somewhere "valid", and sometimes it doesn't. You can fix this by always ensuring your pointers point somewhere valid before using them.