C strcpy segmentation fault when copying from another function

Question

This is a snippet of the code from a project made in the programming class at my college, and my problem is that I get a segmentation fault error when I get to the strcpy part and I have no idea why.

I don't know if it's relevant or not, but I am coding in vs code under linux.

Here's the code:

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

typedef struct Teams {
    char country[20];
    char sponsor[20];
    char group;
    int points;
} E;

char *sponsor_generator(char *country) {
    int i, k = 0;
    char sp[20];
    for (i = strlen(country) - 1; i >= 0; i--) {
        sp[k] = country[i];
        k++;
    }
    sp[k] = '\0';
    return sp;
}

void read(E *ec, int *n) {
    (*n)++;
    printf("Country: ");
    scanf("%s", (ec + *n)->country);
    (ec + *n)->group = (ec + *n)->country[0];
    do {
        printf("Number of points: ");
        scanf("%d", &(ec + *n)->points);
    } while ((ec + *n)->points >= 10);

    strcpy((ec + *n)->sponsor, sponsor_generator((ec + *n)->country));
}

int main() {
    int n = -1;
    E ec[64];
    read(ec, &n);
    return 0;
}

I tried to look up any solutions, but I didn't find something that would work.

In `sponsor_generator` the variable `sp` is a *local* variable. When you return from the function the variable will cease to exist. The pointer you return will become invalid, and any attempt to dereference it will lead to *undefined behavior*. — Some programmer dude, Nov 23 '22 at 08:28
And why use explicit pointer arithmetic like `(ec+*n)`? For any pointer or array `ec` and index `*n` the expression `*(ec+*n)` is *exactly* equal to `ec[*n]`. — Some programmer dude, Nov 23 '22 at 08:29
Compile with all warnings enabled and treat the warninsg as errors — Jabberwocky, Nov 23 '22 at 08:30
`return sp;` return a local variable (i.e. a variable with automatic storage duration). That leads to undefined behavior in C, i.e. an implementation may do whatever it wants. For instance gcc will in some (perhaps all ??) cases return a NULL pointer instead. In that case `strcpy` will be called with NULL which leads to bad, bad things. See this https://ideone.com/DriqoE for a simple example. — Support Ukraine, Nov 23 '22 at 08:33
Thank you, I solved it by turning it into a pointer and allocating memory that way. This makes so much sense now. — Hiroi, Nov 23 '22 at 08:38
I recommend you pass in a buffer (and its max size) as arguments instead. — Some programmer dude, Nov 23 '22 at 08:57

chqrlie · Answer 1 · 2022-11-23T09:25:35.373

There are multiple problems in your code:

function sponsor_generator returns a pointer to a local array that become invalid as soon as the function returns. sponsor_generator should take the destination array as an argument.
naming the next function read probably clashes with the POSIX system call by the same name. Use a different name.

scanf("%s", (ec + *n)->country); may cause a buffer overflow if the input exceeds 19 characters. Always specify a limit and test the return value:

  if (scanf("%19s", (ec + *n)->country) != 1) {
      printf("premature end of file\n");
      exit(1);  // or return an error code to the caller
  }

using the array syntax would make the code more readable.
naming the type E is a bad idea: using the structure tag Team would improve readability.
passing the address of the array element and relying on the function return value to increment the number of entries simplifies the code.

Here is a modified version:

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

typedef struct Team {
    char country[20];
    char sponsor[20];
    char group;
    int points;
} Team;

char *sponsor_generator(char *sp, const char *country) {
    int i, k;
    for (i = strlen(country), k = 0; i-- > 0; k++) {
        sp[k] = country[i];
    }
    sp[k] = '\0';
    return sp;
}

/* flush the remainder of the input line:
   return EOF at end of file or on read error
   return '\n' otherwise
 */
int flush_line(FILE *fp) {
    int c;
    while ((c = getc(fp)) != EOF && c != '\n')
        continue;
    return c;
}

int read_team(Team *team) {
    int c, points, res;

    printf("Country: ");
    res = scanf("%19s", team->country);
    flush_line(stdin);
    if (res != 1) {
        return -1;
    }

    for (;;) {
        printf("Number of points: ");
        res = scanf("%d", &points);
        c = flush_line(stdin);
        if (res != 1) {
            printf("invalid input\n");
            if (c == EOF) {
                return -1;
            }
        } else {
            if (points >= 0 && points < 10)
                break;
            printf("number of points must be between 0 and 9\n");
        }
    }
    team->points = points;
    team->group = team->country[0];
    sponsor_generator(team->sponsor, team->country);
    return 0;
}

void print_team(const Team *team) {
    printf("country: %s, sponsor: %s, group: %c, points: %d\n",
           team->country, team->sponsor, team->group, team->points);
}

int main() {
    Team ec[64];
    int n;

    for (n = 0; n < 64; n++) {
        if (read_team(&ec[n]) < 0) {
            printf("premature end of file\n");
            break;
        }
    }
    for (int i = 0; i < n; i++) {
        print_team(&ec[n]);
    }
    return 0;
}

C strcpy segmentation fault when copying from another function

1 Answers1