MALLOC function usage prob

How is my usage of malloc wrong??

Code:

typedef struct stack {
	char data[SIZE_MAX]; // SIZE_MAX #defined as 100
	int size;
} stack;

stack *s=(stack*)malloc(sizeof(stack));  //line# 11

output:

Code:

prg1.c:11: error: initializer element is not constant

I think that this is because C doesn't allow to use non-constants for initialization of global variables. The pointer that malloc returns is naturally not a compile-time constant.

Give us more code. I believe the above error could occur if you call malloc in the global scope (outside main and/or another function). Is that the case? The global variables are initialized in compile time, not at run time, so you cannot use malloc to intialize

here's the code

Code:

...
#define SIZE_MAX 100

typedef struct stack {
	char data[SIZE_MAX];
	int size;
} stack;

stack *s=(stack*)malloc(sizeof(stack));

void init_stack() {
	s->size=0;
	return;
}

void push(char c) {
	if(s->size<SIZE_MAX) {
		s->data[(s->size)++]=c;
	}
	else printf("\nStack Full!\n");
	return;
}

char pop() {
	if(s->size==0) {
		printf("\nStack Empty!\n");
		return('~');
	}
	else return s->data[--(s->size)];
}
int main(void) {
	int t,i,j=0;
	char expressions[100][400],ch;
	printf("no. of expressions:");
	scanf("&#37;d",&t);
	printf("enter the expressions");
	for(i=0;i<t;i++) {
		printf("expr[%d]:",i+1);
		scanf("%s",expressions[i]);
	}

	for(i=0;i<t;i++) {
		init_stack();
		while(j<strlen(expressions[i])) {
			ch=expressions[i][j++];
			if(isalpha(ch)) {
				printf("%c",ch);
			}
			else if(ch==')') {
				ch=pop();
				if(ch!='~') printf("%c",ch);
			}
			else if(ch=='+'||ch=='-'||ch=='*'||ch=='/'||ch=='^') {
				push(ch);
			}
		}
	}
	return 0;
}

As anon says, it's a global variable, they can not be calling functions to initialize their value.

In this particular case, it looks like you only ever have one stack, so why not just make it a global variable, not a pointer - there is no benefit here to have a pointer (it just uses a bit more memory, and makes the code longer - I don't see any benefit in that - and global variables can be almost as large as you like).

--
Mats

So, the correct way to dynamically allocate memory for a global variable is this:

Code:

....
stack *s;

int main(void) {
        ...
        s = (stack *)malloc(sizeof(stack));
        ...
}

But, as matsp mentioned you could just do this:

Code:

....
stack s;

int main(void) {
    ...
}

and replace of course "s->" with "s.". There is typically a difference in the two ways, but in your program there is no need to dynamically allocate memory. It would make sense only if you where allocating memory for the "stack.data[]" array based on a user input, for example, and not a fixed SIZE_MAX number.

The correct way is not to cast the return of malloc when compiling as C.

Originally Posted by Elysia

The correct way is not to cast the return of malloc when compiling as C.

Why is that? I though you would get the same result, but it was typically better to cast the return value of malloc()

It's in the FAQ.

Yes, as matp and others said malloc should not be used in global scope.
I've re-arranged your code to make it work...!

Code:

...
#define SIZE_MAX 100

typedef struct stack 
{
	char data[SIZE_MAX];
	int size;
} stack;

stack *s;

void init_stack() 
{
	s->size=0;
	return;
}

void push(char c) 
{
	if(s->size<SIZE_MAX) 
	{
		s->data[(s->size)++]=c;
	}
	else 
		printf("\nStack Full!\n");
	return;
}

char pop() 
{
	if(s->size==0) 
	{
		printf("\nStack Empty!\n");
		return('~');
	}
	else 
		return s->data[--(s->size)];
}
int main(void) 
{
	int t,i,j=0;
	char expressions[100][400],ch;

	s = (stack *) malloc(sizeof(stack));
	printf("no. of expressions:");
	scanf("%d",&t);
	printf("enter the expressions");
	for(i=0;i<t;i++) 
	{
		printf("expr[%d]:",i+1);
		scanf("%s",expressions[i]);
	}

	for(i=0;i<t;i++) 
	{
		init_stack();
		while(j<strlen(expressions[i])) 
		{
			ch=expressions[i][j++];
			if(isalpha(ch)) 
			{
				printf("%c",ch);
			}
			else if(ch==')') 
			{
				ch=pop();
				if(ch!='~') printf("%c",ch);
			}
			else if(ch=='+'||ch=='-'||ch=='*'||ch=='/'||ch=='^') 
			{
				push(ch);
			}
		}
	}
	return 0;
}

Still, why do you need to use malloc?
You are also committing two sins.
First, you cast the return of malloc. Do not do this. This was explained earlier!
And two, you are not freeing what you malloc.

First, you cast the return of malloc. Do not do this. This was explained earlier!

Could you tell in detail for not to cast the malloc result.
I feel we should, since the malloc returns void pointer.
I've been doing this and it works well with C...!

Originally Posted by patil.vishwa

Could you tell in detail for not to cast the malloc result.
I feel we should, since the malloc returns void pointer.
I've been doing this and it works well with C...!

And it will continue to do so, until the day when it suddenly stops. But: all those things in red here are links -- so when people say "It's in the FAQ", and the word FAQ is red, that means you can click on it to go to that entry in the FAQ.

It's explained very well in the FAQ! It's there for a reason.

Originally Posted by patil.vishwa

Could you tell in detail for not to cast the malloc result.
I feel we should, since the malloc returns void pointer.
I've been doing this and it works well with C...!

But a void pointer (in C, but not C++) can be automatically, without a cast, converted to any other pointer type:

Code:

int x;
void *p = &x;
int *pi = p;
double *pd = p;

The above code will compile correctly - but of course, if you actually write something through pd, it will most likely cause all sorts of "interesting" side effects.

But that's not the reason you shouldn't cast malloc. The reason is that without a cast, compilers will notice the fact that you have not included the correct header-file for malloc. This can be more critical in some compilers than others - the example I usually give is a 68K compiler that returns a pointer in A0, and integer types in D0. If you miss out the prototype for malloc, then the compiler will assume that it returns an int. It is permissible by code to cast an int into a void *, so the compiler will happily use whatever is in D0 to form your pointer. Unfortunately, the D0 value may well be a sort of sane address [because quite often, there is a bit of "calculation" going on inside the malloc function, which may well cause D0 to be loaded with an integer value representing nearly the right address - this would cause the system to crash only when you try to free the block or some such - which may be many many lines from where it went wrong. And the difference between debugging the problem with a compiler error/warning, and debugging a "some memory used by malloc and free is messed up is a factor of 100 in difficulty - the compiler will point at exactly the right line, whilst free will just crash/misbehave in some rather random way (if you are really unlucky, it is not the first, second or third call to free that crashes either - but something else somewhere else completely, so you don't even know that it's free that is causing the problem - it could take someone fairly experienced days to figure that one out)].

I'm sure there are other compilers that return pointers in a different way than integer values (64-bit systems are also in this category, for example - and again, a cast may hide the problem for quite some time, so the problem only gets visible when you allocate enough memory to get over a 2GB boundary, for example).

So in summary: It's a case of converting a potential bug into a compiler error/warning.

--
Mats