print an array of structs, C

Question

I wanna print my array of structs but i keep getting segmentation fault, what am i doing wrong? I start by using another function to read in data. Then i wanna be able to view what i just typed in.

#include <stdio.h>

#define max 100
#define min 30
struct vara{
    char namn[min];
    int saldo;
    int nummer;
};
struct vara varor[max];

int addvara(struct vara varor[], int length)
{
    int s, n = 1;
    //char nm;
    printf("Vill du scanna in vara? 1 för ja. 0 för att återgå till menyn\n");
    scanf("%d", &n);
    while(n != 0)
    {
        for(int i=0; i<length; i++)
        {
            printf("Namm, saldo, varunummer\n");
            scanf("%s", varor[i].namn);
            scanf("%d", &varor[i].saldo);
            scanf("%d", &varor[i].nummer);
            //struct vara invara = {"%s, %d, %d", nm, s, n};
            //varor[length] = invara;
            i=length;
            length++;
            printf("Vill du scanna in vara? 1 för ja. 0 för att återgå till menyn\n");
            scanf("%d", &n);
        }

    }
    return length;
}

void view(struct vara varor[], int length)
{
    printf("Varunummer \t\t Namn \t\t Varunummer\n");
    for(int i=0; i<length; i++)
    {
        printf("%s \t\t %d \t\t %d", varor[i].namn, varor[i].saldo, varor[i].nummer);
    }
}

Can't be diagnosed from this sample. Show us some actual running code. — Lee Daniel Crocker
– Lee Daniel Crocker, Commented Mar 2, 2018 at 19:29
The unconditional i=length; within the body of for(int i=0; i<length; i++) makes no sense. You then increment length so that's an infinite loop? Please post the Minimal, Complete, and Verifiable example that shows the problem. — Weather Vane
– Weather Vane, Commented Mar 2, 2018 at 19:30
Your addvara is confusing, how do you increment length? You are most probably indexing the array out of bound which is undefined behaviour and a segaullt is the consequence. — Pablo
– Pablo, Commented Mar 2, 2018 at 19:30
"what am i doing wrong?" Code is not checking the return values from scanf(). example: scanf("%s", varor[i].namn); --> if (scanf("%29s", varor[i].namn) != 1) Handle_Error();. Like wise for the other scanf(). — chux
– chux, Commented Mar 2, 2018 at 19:35
hmm i see i need to change somethings i will post edited code soon. — Tobias Haraldsson
– Tobias Haraldsson, Commented Mar 2, 2018 at 20:06

Pablo · Accepted Answer · 2018-03-03 00:21:43Z

I think the root of the problem is your addvara function, your loops don't make sense.

With i=length; you are overwriting the running index, then you do length++;. If the user wishes to continue, you exit the for loop and enter it again, depending on the number of times the user inputs the values, you are going to eventually access varor out of bounds, which is undefined behaviour and a segfault is a consequence of that.

I'd rewrite the addvara function like this:

// helper function to clean stdin
void clean_stdin()
{
    int c;
    while((c = getchar()) != '\n' && c != EOF);
}

// helper function to read in a loop until you get the value
// return 1 on success, 0 on failure, cannot continue reading
int read_from_stdin(const char *format, void *ptr, const char *prompt)
{
    int err;
    do {
        if(prompt)
        {
            printf("%s", prompt);
            fflush(stdout);
        }

        err = scanf(format, ptr);

        if(err == EOF)
            return 0;

        if(err != 1)
        {
            printf("Invalid value, try again\n");
            clean_stdin();
        }

    } while(err != 1);

    return 1;
}

int addvara(struct vara varor[], int length)
{

    if(varor == NULL)
        return -1; // error value

    if(length < 0)
        return -1;

    // helper to creata a format for scanf
    // %29[^\n]s, the number depends on sizeof varor[0].namn
    char strfmt[100];
    sprintf(strfmt, "%%%zu[^\n]s", sizeof(varor[0].namn) - 1);

    int i;
    for(i = 0; i < length; ++i)
    {
        int ask;

        if(read_from_stdin("%d", &ask, "Vill du scanna in vara? 1 för ja. 0 för att återgå till menyn\n") == 0)
        {
            fprintf(stderr, "cannot continue reading\n");
            return i; // number of values added so far
        }

        if(ask == 0)
            return i; // the number of values added

        // otherwise scanf might complain about leftovers in the input buffer
        clean_stdin();


        if(read_from_stdin(strfmt, varor[i].namn, "Namm: ") == 0)
        {
            fprintf(stderr, "cannot continue reading\n");
            return i; // number of values added so far
        }

        // otherwise scanf might complain about leftovers in the input buffer
        clean_stdin();

        if(read_from_stdin("%d", &varor[i].saldo, "Saldo: ") == 0)
        {
            fprintf(stderr, "cannot continue reading\n");
            return i; // number of values added so far
        }

        if(read_from_stdin("%d", &varor[i].nummer, "Varunummer: ") == 0)
        {
            fprintf(stderr, "cannot continue reading\n");
            return i; // number of values added so far
        }

        putchar('\n');
    }

    return i;
}

Your printing function looks OK. But you could improve how your format your output, for example:

void view(struct vara varor[], int length)
{
    printf("%-30s %-10s %s\n", "Namn", "Saldo", "Nummer");

    for(int i=0; i<length; i++)
        printf("%-30s %-10d %d\n", varor[i].namn, varor[i].saldo, varor[i].nummer);
}

The output would look like this

Namn                           Saldo      Nummer
Jan                            129        1113232
Maria                          232        44342234

edit

As chux pointed out in the comments, my read_from_stdin function has technically undefined bevahiour, because scanf expects a int* pointer for the %d format and I'm using a void* pointer.

I'd like to quote chux here:

chux wrote in the comments

"%d" expects a int *, not a void *. A void * and int* could be of different widths, encodings

However after 15 year of writing C code, I've never come across a commonly used architecture where this is the case and a function like this would actually fail. Unless your target architecture isn't an exotic one, this function would work as I intended.

ok great thanks!! Im still going trough your code trying to understand everything. When im testing it out it seems to work fine except the output prints a lot of zeros. can print a screenshot tomorrow.
void *ptr ... scanf(format, ptr); is technical UB when format is "%d", yet I am certain it will work as desired on all platforms you try.
@chux can you please elaborate more on why it is technically UB? Are there really platforms out there where this would fail?
"%d" expects a int *, not a void *. A void * and int* could be of different widths, encodings, etc. "Are there really platforms out there where this would fail? " I doubt it.
@chux thanks for the feedback, I've added a warning about my read_from_stdin function.

Collectives™ on Stack Overflow

print an array of structs, C

1 Answer 1

5 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Related