longest string array segmentation fault

Question

Write a C function longestStrInAr() that takes in an array of strings str and size (>0) as parameters, and returns the longest string and also the length of the longest string via the pointer parameter length. If two or more strings have the same longest string length, then the first appeared string will be returned to the calling function. For example, if size is 5 and the array of strings is { "peter", "john", "mary", "jane", "kenny"}, then the longest string is "peter" and the string length is 5 will be returned to the calling function

I am getting a segmentation fault here and I don't know why.

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

#define N 20

char *longestStrInAr(char str[N][40], int size, int *length);

int main() {
    int i, size, length;   
    char str[N][40], first[40], last[40], *p, *result;
    char dummychar;   
    
    printf("Enter array size: \n");
    scanf("%d", &size);
    scanf("%c", &dummychar);
    for (i = 0; i < size; i++) {
        printf("Enter string %d: \n", i + 1);
        fgets(str[i], 40, stdin);
        if (p = strchr(str[i], '\n'))
            *p = '\0';   
    }  
    result = longestStrInAr(str, size, &length);
    printf("longest: %s \nlength: %d\n", result, length);         
    return 0;
}

char *longestStrInAr(char str[N][40], int size, int *length) {
    char *p;
    for (int i = 0; i < size; i++) {
        int j = 0; int max = 0, *length = 0;
        while (str[i][j++] != '\0') {
            max++;
        }
        if (max > *length) {
            *length = max;
            p = str[i];
        }
    }
    return p;
}

Have you tried running your code line-by-line in a debugger while monitoring the values of all variables, in order to determine in which line your program stops behaving as intended? If you did not try this, then you may want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) You may also want to read this: [How to debug small programs?](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) — Andreas Wenzel, Jun 19 '22 at 14:21

score 1 · Answer 1 · answered Jun 19 '22 at 14:03

1

The line

int j = 0; int max = 0, *length = 0;

is bad. A new pointer variable length is declared here and initialized to NULL, shadowing the argument length.
After that, this length (NULL) is dereferenced and it will cause Segmentation Fault.

To initialize what is pointed at by the argument length to zero, the line should be:

int j = 0, max = 0; *length = 0;

answered Jun 19 '22 at 14:03

MikeCAT

73,922
11
45
70

I don't think that it is good programming style to have a statement on the same line as a declaration. – Andreas Wenzel Jun 19 '22 at 14:29
@AndreasWenzel I agree. I also think that the better place for `*length = 0;` may be *before* the `for` loop. – MikeCAT Jun 19 '22 at 14:44
2

"To initialize what is pointed at by the argument length to zero, the line should be:" -- This sentence gives the impression that you are recommending to put a statement on the same line as a declaration. You may want to elaborate a bit in your answer. – Andreas Wenzel Jun 19 '22 at 14:48
1

took me a long time to see the difference between those 2 lines. split them up – pm100 Jun 19 '22 at 17:01

chqrlie · Answer 2 · 2022-06-19T20:16:35.557

In function longestStrInAr, the code in the for loop is incorrect:

    int j = 0; int max = 0, *length = 0;

defines 3 local variables: j, max and length as a new pointer to int, initialized as a null pointer. This length variable has the same name as the function argument but is defined in the scope of the block that is the body of the for statement, hence it shadows the argument with the same name. This problem would have been diagnosed by the compiler if given a higher level of warnings (gcc -Wall -Wextra -Wshadow, clang -Weverything).

This is causing the segmentation fault: when you later dereference this pointer in if (max > *length), you have undefined behavior as length is a null pointer.

You probably meant to just use the argument and initialize *length, and you should do this before the for loop. Note also that you should initialize p as p = str[0] for the pathological case where all strings are empty for which the posted code returns an uninitialized pointer. As a matter of fact, there is another special case to consider: if size is 0, the function should return NULL.

There are some problems in the main function too: if size is greater than N, you read strings beyond the end of the array.

Here is a modified version:

#include <stdio.h>

#define N 20

char *longestStrInAr(char str[][40], int size, int *length);

int flush_input(void) {
    int c;
    while ((c = getchar()) != EOF && c != '\n')
        continue;
    return c;
}

int main() {
    int i, size, length;   
    char str[N][40];
    char *result;
    
    printf("Enter array size: \n");
    if (scanf("%d", &size) != 1 || size < 0 || size > N) {
        fprintf(stderr, "invalid input\n");
        return 1;
    }
    // read and discard the rest of the input line
    flush_input();
    for (i = 0; i < size; i++) {
        printf("Enter string %d: \n", i + 1);
        str[i][0] = '\0';
        scanf("%39[^\n]", str[i]);
        flush_input();
    }  
    result = longestStrInAr(str, size, &length);
    printf("longest: %s \nlength: %d\n",
           result ? result : "(null)", length);         
    return 0;
}

char *longestStrInAr(char str[][40], int size, int *length) {
    char *p = NULL;
    *length = 0;
    if (size > 0) {
        for (int i = 0; i < size; i++) {
            int j = 0;
            while (str[i][j] != '\0')
                j++;
            if (*length < j) {
                *length = j;
                p = str[i];
            }
        }
    }
    return p;
}

@AndreasWenzel: I agree, answer updated. – chqrlie Jun 19 '22 at 20:16 — chqrlie, Jun 19 '22 at 20:16

longest string array segmentation fault

2 Answers2