-2

I cannot understand what's wrong with my c implementation of: dynamically allocating an array of struct inside a function to use with in other functions.

The problem is my .exe stops working after reading the first struct(which is read correctly).

The struct:

struct student
{
    char name1[30], name2[30];
    float grade;
};

The function:

void read(int *n, struct student **a)
{
    if(scanf(" %d", n) == 1)
    {
        int i;
        *a=(struct student*)malloc((*n)*sizeof(struct student*));
        for(i=0; i<*n; i++)
            scanf("%29s %29s %f",(*a)[i].name1, (*a)[i].name2, &(*a)[i].grade);
            //it stops working right after this line is executed
    }
}

Main:

int main()
{
    int n;
    struct student *a;
    read(&n, &a);
    return 0;
}

Warrnings:

format '%s' expects argument of type 'char *', but argument 2 has type 'char (*)[30]' [-Wformat=]|

format '%s' expects argument of type 'char *', but argument 3 has type 'char (*)[30]' [-Wformat=]|

Using a+i instead of a[i] doesn't change anything. I am aware &(*a) means a, but i wanted to make everything as clear as possible. I feel like there's something obviously wrong about my dynamic allocation that i'm missing. I read so many questions here, but nothing appears to solve my problem. Thanks for your time!

EDIT 1: i changed the code to the suggestions:

scanf("%29s %29s %f", a[i].name1, a[i].name2, a[i].grade);

and now i get the error below instead.

Error:

error: request for member 'name1' in something not a structure or union

EDIT 2: so, the line:

 scanf("%29s %29s %f",*a[i].name1, *a[i].name2, *a[i].grade);

gives the error:

request for member 'name1' in something not a structure or union

and the line:

scanf("%29s %29s %f",(*a)[i].name1, (*a)[i].name2, (*a)[i].grade);

crashes.

EDIT 3:

scanf("%29s %29s %f", (*a)[i].name1, (*a)[i].name2, &(*a)[i].grade);

works.

  • 3
    It's an issue about [*operator precedence*](http://en.cppreference.com/w/c/language/operator_precedence). Using your code the expression `*a[i]` is equal to `*(a[i])` which isn't correct. – Some programmer dude Jan 12 '17 at 14:39
  • 1
    This is the first time, and hopefully the last time, I've ever seen someone write &(*a). – jarmod Jan 12 '17 at 14:40
  • 1
    This: `scanf("%d",&(*n));` is just scary, it should of course be `if(scanf(" %d", n) == 1)`, no need to dereference and then take the address when *you have the address to begin with*. Also I/O can fail, and that space before `%d` is very good to have. – unwind Jan 12 '17 at 14:41
  • `((*a)[i]).name2` (additional brackets: @Someprogrammerdude is right) is a char array which is adjusted to a pointer to its first element when used as a function argument. Taking the address of the array doesn't change the numeric value (because both the address of the first element of an array and the address of the array proper are the address of the first byte of the first element), but it changes the *type*, to a pointer to array. Just leave away the address operator, and you will pass a pointer to the first char in `name2` which is what you want. – Peter - Reinstate Monica Jan 12 '17 at 14:44
  • @Someprogrammerdude While your comment is factually correct it is not the reason for the compiler's comment but would simply crash at run time. Indexing is, after all, one form of dereferencing, and in which order the two necessary dereferencings happen is grammatically irrelevant. Edit: Oh, I see the OP was also asking about the run time crash. Never mind... – Peter - Reinstate Monica Jan 12 '17 at 14:49
  • @PeterA.Schneider Yes it would probably crash at run-time, ***but*** it also leads to the wrong type being passed to the `scanf` function which is exactly what the compiler is complaining about. – Some programmer dude Jan 12 '17 at 14:51
  • @Someprogrammerdude I thought not, but I would defer to authorities... Edit: I shouldn't -- the resulting type would be the same. After all, `*` is equivalent to `[0]`, so `*a[1]` can be equivalently written `a[1][0]`, which has the same type as `a[0][1]`, obviously. – Peter - Reinstate Monica Jan 12 '17 at 14:53
  • @jarmod I take it that you have not read the C standard. – Peter - Reinstate Monica Jan 12 '17 at 15:01
  • Ok. I will never use &(*a), it's just a pleonasm and i changed 'scanf("%d",&(*n))' to 'if(scanf(" %d", n) == 1)'. @Mgetz, i read the article, but, it's not the same thing because the field of the struct: a[i].name1 is already a pointer that i don't dereference(and his returns an error, mine doesn't). I changed to his fix anyway just to try, and it doesn't work because **error: request for member 'name1' in something not a structure or union**. Thanks for the answers. –  Jan 12 '17 at 15:04
  • @SurrealEverything you still need to dereference `a`, you just don't need the `&` I would suggest assigning to a local pointer and only ever assigning to `a` at the very end of the function. Doing so would dramatically simplify your code. – Mgetz Jan 12 '17 at 15:10

1 Answers1

1

Here

*a=(struct student*)malloc((*n)*sizeof(struct student*));
                                                   ^^^^^

you allocate space for *n pointers to struct student but it seems that you really want to allocate space for *n struct student.

It seems you want:

*a=malloc((*n)*sizeof(struct student));

Also notice that *a[i] is the same as *(a[i]) but you probably want (*a)[i]. So you need something like:

scanf("%29s %29s %f", (*a)[i].name1, (*a)[i].name2, &(*a)[i].grade);

Notice that you need & in front of (*a)[i].grade but not the other two places because the two two other are arrays.

As mention by @unwind in a comment: The scanf is wrong

This

scanf("%d",&(*n));

should be

scanf("%d", n);

and then you should also check the return value, like

if (scanf("%d", n) != 1)
{
    // Add error handling here
    ....
}
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63