0

Program:

#include <stdio.h>
#include <stdlib.h>
struct s
{
    int a;
    struct x
    {
        int b;
    }*p;
};
int main()
{
    struct s *a;
    a=(struct s *)malloc(sizeof(struct s) * 10);
    a->p=(struct x)malloc(sizeof(struct x));
    return 0;
}

Error:- In function 'main': 16:41: error: conversion to non-scalar type requested a->p=(struct x)malloc(sizeof(struct x)); ^

Jens
  • 69,818
  • 15
  • 125
  • 179
Satender
  • 117
  • 1
  • 2
  • 9
  • 1
    `=(struct x)malloc` -> `=(struct x*)malloc`. Or just don't cast (e.g.: `a->p = malloc(sizeof(struct x));` – UnholySheep Apr 28 '17 at 14:23
  • 3
    [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh Apr 28 '17 at 14:24
  • Third the "just don't cast." [Really, just don't](http://c-faq.com/malloc/mallocnocast.html). – Cong Ma Apr 28 '17 at 14:26

6 Answers6

2

In C, as in physics, a scalar type is a type that just has a single magnitude. Integer types, floating point types and pointer types are considered to be scalar types. structs are not scalar types.

This line

 a->p=(struct x)malloc(sizeof(struct x));

attempts to cast the result of a malloc which is a pointer (a void* in fact and thus a scalar, to a struct which is not. a->p is also a pointer, having type struct s* so what the error message means is that your cast forgets the * that makes the target type a pointer type. i.e. this is what it should be:

 a->p=(struct x*)malloc(sizeof(struct x));

Actually, in C, as opposed to C++ you don't need to cast void* to other pointer types, so it is better just to write

 a->p=malloc(sizeof(struct x));

The main reason for this is that if you forget to include stdlib.h which is where the prototype for malloc comes from, the compiler assumes that malloc returns int which is a disaster if a pointer cannot be represented as an int. For example, modern 64 bit compilers often have 32 bit int and 64 bit pointers. Putting the cast in silences the warning that the compiler is assuming malloc returns int because you forgot stdlib.h

One final improvement: you can take sizeof of an object even if it doesn't exist yet so

a->p=malloc(sizeof *(a->p));

The compiler often knows the size of the thing a->p points to and so sets the sizeof to the right number as long as it knows the size (see "incomplete types" for an exception). Now you can modify the type of a->p without having to fix all the mallocs.

JeremyP
  • 84,577
  • 15
  • 123
  • 161
0

To avoid type-mismatches, allocations should follow the canonical way of

a = malloc(10 * sizeof(*a));
a->p = malloc(1 * sizeof(*a->p));

i.e. always referencing the variable being assigned to in the sizeof. Note how short this is. Note how this auto-adapts when the type of a changes. You could even drop the parentheses around the sizeof operand and the multiplication by 1.

Jens
  • 69,818
  • 15
  • 125
  • 179
0

As stated in the comments, you're attempting to assign a struct x (casted from void *) to a struct x *. You can't assign a non-pointer to a pointer.

You can avoid this by using the correct cast:

a->p=(struct x *)malloc(sizeof(struct x));

Or better yet remove the cast entirely, since you can freely cast between a void * and any non-function pointer without one:

a=malloc(sizeof(struct s) * 10);
a->p=malloc(sizeof(struct x));
dbush
  • 205,898
  • 23
  • 218
  • 273
0

Missing asterix. If the type conversion is really necessary, the statement

a->p=(struct x)malloc(sizeof(struct x));   // struct x is not a pointer

should actually be

a->p=(struct x *)malloc(sizeof(struct x));

Of course, as others have said in comments, the type conversion is inadvisable. It would also be advisable to not name the types being malloc()ed in the parameter of sizeof so the two statements in your function would be

a = malloc(sizeof(*a) * 10);
a->p = malloc(sizeof(*(a->p)));

It would also be advisable to check the result of each call of malloc() before using it, since malloc() returns NULL when it fails.

Note: if your C compiler is really a C++ compiler, a conversion of the result of malloc() would be needed. However, in standard C, the conversion is not required, and is actively discouraged.

Peter
  • 35,646
  • 4
  • 32
  • 74
0

You have a simple typo that originates from the inadvisable practice of casting malloc on the right of an assignment1.

Also you should check the return value of malloc, especially before using the pointer to member operator ->.

So I'd rather see something on the lines of

if (a = malloc(sizeof(*a) * 10)){
    if (a->p = malloc(sizeof(*(a->p)))){
        // Everything is allocated.
        // ToDo - code here
        free(a->p);
    }
    free(a);
}

(Note that in the evaluation of sizeof(*(a->p)) the compiler must not actually do any dereferencing, so it doesn't matter if a is valid or a->p is valid.)


1See Do I cast the result of malloc?

Community
  • 1
  • 1
Bathsheba
  • 231,907
  • 34
  • 361
  • 483
-1

You should try a->p=(struct x*)malloc(sizeof(struct x));

Pras
  • 4,047
  • 10
  • 20