Malloc problem

[Dazaa]

Hi,

I usually use C++, but I am working with some very old code and need to do it in C.

So i understand malloc is used
char *p;
p = malloc(200);

where 200 is the amount of bytes I need.
and yes i know I need to use free.

But I was trying to allocate an array to read a file in. and I am getting this compiler error:

test.c:13: error: invalid conversion from `void*' to `char*'

I have absolutely no idea whats wrong. Can anyone please explain?

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

int main() {
   char *pa[1000],buffer[128];
   FILE *f;
   int num = 0;

   f = fopen("file1_Windows.txt","r");

   while (fgets(buffer,128,f) != NULL) {
      pa[num] = malloc(128);
      strcpy(pa[num],buffer);
      num++;
   }
	printf("%s", pa[0]);
}

[sree_ec]

Hi,

I usually use C++, but I am working with some very old code and need to do it in C.

So i understand malloc is used
char *p;
p = malloc(200);

where 200 is the amount of bytes I need.
and yes i know I need to use free.

But I was trying to allocate an array to read a file in. and I am getting this compiler error:

test.c:13: error: invalid conversion from `void*' to `char*'

I have absolutely no idea whats wrong. Can anyone please explain?

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

int main() {
   char *pa[1000],buffer[128];
   FILE *f;
   int num = 0;

   f = fopen("file1_Windows.txt","r");

   while (fgets(buffer,128,f) != NULL) {
      pa[num] = malloc(128);
      strcpy(pa[num],buffer);
      num++;
   }
	printf("%s", pa[0]);
}

See malloc

void *malloc(size_t size);

It returns void*.
So use a type casting to (char*) before you assign..

But I guess you use a very old version of compiler. Newer compilers wont even care to give a warning for this...

[Dazaa]

I compiled with:
pa[num] = (char *)malloc(128);
seems to work.

[Dazaa]

Yeh i think the compiler is about a year or 2 old.
Its a C++/C compiler. gcc

[mitrmkar]

>> the compiler is about a year or 2 old.
>> Its a C++/C compiler. gcc

Most likely the code is not a compiled as C, but C++ instead. In C, you don't need to cast return value from malloc(), and moreover, you shouldn't do that.

[sree_ec]

>> the compiler is about a year or 2 old.
>> Its a C++/C compiler. gcc

Most likely the code is not a compiled as C, but C++ instead. In C, you don't need to cast return value from malloc(), and moreover, you shouldn't do that.

Yes you are right.. C++ compiler could be the villain....

[N1GHTS]

> In C, you don't need to cast return value from malloc(), and moreover, you shouldn't do that.

I looked over that link attached to your warning that it shouldn't be done. Its just saying that it helps prevent problems when you dont include a file. Is that a strong enough reason not to cast your malloc results to your target? I always cast results so the compiler knows my destination size. It may not always be necessary but I think its good practice to cast as a way to help the compiler out. Sometimes the compiler can be stupid.

[Narue]

>Is that a strong enough reason not to cast your malloc results to your target?
No, but I find not needing to type out a redundant cast and not needing to mention the target data type at all somewhat compelling:

p = malloc(N * sizeof *p);

>I always cast results so the compiler knows my destination size.
That's the kind of statement I expect to see from someone who doesn't understand how pointers to void work. I'm kind of surprised, to be honest. You seem like you know what you're doing.

>It may not always be necessary
The only time it's necessary to cast malloc in standard C is when writing code that needs to be compilable as both C and C++, and that's a rather small niche.

>Sometimes the compiler can be stupid.
Not that stupid. Many bad practices have been propagated far longer than necessary in the name of "helping" the compiler when the compiler doesn't need any help. One of my pet peeves is programmers who think compiler writers are incompetent.

[N1GHTS]

> I find not needing to type out a redundant cast and not needing to mention the target data type at all somewhat compelling
> The only time it's necessary to cast malloc in standard C is when writing code that needs to be compilable as both C and C++

I strongly believe the more details you enter in your code, the less ambiguous your meaning is, especially when working with open source code where others will try to interpret your meaning to various things. Additionally, any unnecessary casts are ignored by the compiler. To me its all about "best practice" and it seems that omission without any added benefit is like encrypting variable names instead of make them meaningful. Please correct me if i'm wrong with my thought process here.

One of my pet peeves is programmers who think compiler writers are incompetent.

By no means do I believe that the compiler is incompetent. I just don't think it can always correctly interpret my meaning if I chose the path of assumptions and ambiguity for the sake of saving a few extra characters of code.

I'm kind of surprised, to be honest. You seem like you know what you're doing.

Not sure what your trying to prove and who your trying to prove it to with this statement. Either way, water under the bridge.

[Narue]

>Please correct me if i'm wrong with my thought process here.
As long as you recognize that your argument boils down to "I like it this way because it makes more sense to me", it's all good. It's not my place to dictate personal styles, provided you're aware of the alternatives and make an educated choice.

>I just don't think it can always correctly interpret my meaning if I chose the path
>of assumptions and ambiguity for the sake of saving a few extra characters of code.
The problem here is that there's no ambiguity. If you find a compiler that incorrectly interprets your meaning without casting malloc, I'll show you a severely broken compiler.

>Not sure what your trying to prove and who your trying to prove it to with this statement.
It's a compliment (you strike me as a skilled programmer). Would you rather I said your statement was the kind of stupid thing beginners believe and left it at that?

[N1GHTS]

As long as you recognize that your argument boils down to "I like it this way because it makes more sense to me", it's all good

You are correct, its a preference. Weather I think its better is a matter of pure debate, and in a language like C, its a futile debate since C is all about being completely in control.

The problem here is that there's no ambiguity

I see it more as "consistency". malloc() doesn't need it but other places do, so I like being consistent so I don't end up regretting it when a customer complains about some weird error, and im forced to hunt down the problem in the debug logs only to find that somewhere in the code I was copying a void pointer to an incompatible pointer. If I had casted it, my INTENDED type would have been in the code, and the compiler would have warned me about the attempt to copy one type into another incompatible type. It doesn't happen very often and most of the time I catch it but I highly value consistency.

It's a compliment (you strike me as a skilled programmer). Would you rather I said your statement was the kind of stupid thing beginners believe and left it at that?

I figured that's how you felt but the way you said it was initially interpreted with a different level of respect. Written English suffers from this common issue.

[Dazaa]

I got another problem now.
I have never used realloc before but it seems to cause the program to have a runtime error:

At this stage its basic, I only allocate memory and make it bigger if I need to. But have no idea why this wont work:. Obviously I dont understand how to use realloc properly.

//Input a file into a 2D array. 
//file can have any number of lines. But max line size is restricted to BUFFESIZE

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#define BUFFESIZE 80

char* getlin(char* input, int i); //function for me to use to manage the location in the array.

int main() {
	int linemax = 5;
	char *pa = (char *)malloc(sizeof(char) * linemax);
	char buffer[BUFFESIZE];
   FILE *f;
	
   int n = 0;

   f = fopen("file1_windows.txt","r");

   while (!feof(f)) {
		if (n < linemax) {
			fgets(buffer,BUFFESIZE,f);
			strcpy(pa+(n*(BUFFESIZE * sizeof(char))),buffer);
			n++;
		} else {
			linemax*=2;
			pa = (char*)realloc(pa,sizeof(char) * linemax);
		}
   }
fclose(f);
	
	//blah blah blah
	
free(pa);
	
}
char* getlin(char* input, int i) {
	return (input+(BUFFESIZE * sizeof(char)) * i);
}

[sree_ec]

>Is that a strong enough reason not to cast your malloc results to your target?
No, but I find not needing to type out a redundant cast and not needing to mention the target data type at all somewhat compelling:

p = malloc(N * sizeof *p);

>I always cast results so the compiler knows my destination size.
That's the kind of statement I expect to see from someone who doesn't understand how pointers to void work. I'm kind of surprised, to be honest. You seem like you know what you're doing.

>It may not always be necessary
The only time it's necessary to cast malloc in standard C is when writing code that needs to be compilable as both C and C++, and that's a rather small niche.

>Sometimes the compiler can be stupid.
Not that stupid. Many bad practices have been propagated far longer than necessary in the name of "helping" the compiler when the compiler doesn't need any help. One of my pet peeves is programmers who think compiler writers are incompetent.

That's the kind of statement I expect to see from someone who doesn't understand how pointers to void work

Interesting discussion. While I was reading this, I was looking for some information and I think you people would be able to clarify this.

Could you please elaborate on this? What exactly do you mean?
If I do

int *i = NULL;
i = (int*)malloc(sizeof (int) * 4);

Here,
casting a void pointer to integer pointer means we are telling the compiler that a pointer which has no type or no size should be used as a pointer to integer which has 4 times the size of an integer? Is it so? Can you define this operation?

Also what will happen if I do

i = (int*) malloc(10);

10 bytes allocated . Which wont match with alignment of integers [considering 4bytes is the size of int] . How we will define this? What will be the result of malloc, will this be contguous 10 bytes?

I am really sorry if all these are stupid questions. Afterall, I am a beginner :)

[Dazaa]

what i really want to know is if I use realloc will the array be looked after for me? or will i have to copy everything OUT then call realloc then put it back in... because It doesnt seem to work on the fly.... I thought realloc took care of this because it knew the data type.

[sree_ec]

what i really want to know is if I use realloc will the array be looked after for me? or will i have to copy everything OUT then call realloc then put it back in... because It doesnt seem to work on the fly.... I thought realloc took care of this because it knew the data type.

realloc will take care of the data. But you need to check the return type.
But my question is Why you are reallocating with respect to linemax when you are copying Atleast BUFFSIZE to pa in each strcpy.

[Dazaa]

lets just assume that no lines in the file are longer than 80 characters.
What I am doing is starting off with an array which is 5x buffesize. (enough to store 5 lines).

I don't know how long the file is, so if the file is more than 5 lines then I want to double the size needed at runtime. So linemax gets bigger, and so does my array. Basically linemax is the maximum amount of lines the array can hold.

The problem is it doesnt work.

What do you mean by check the return type? I want it to return a pointer to a character string.

I know there will be some wastage if the data is less than BUFFESIZE.

I know there is a problem with my code I have been racking my brain over it for the last two hours trying to find out which line is causing it to read or write outside of the allocated memory... but I am completely lost. The debugger I got is rubbish.

[sree_ec]

lets just assume that no lines in the file are longer than 80 characters.
What I am doing is starting off with an array which is 5x buffesize. (enough to store 5 lines).

I don't know how long the file is, so if the file is more than 5 lines then I want to double the size needed at runtime. So linemax gets bigger, and so does my array. Basically linemax is the maximum amount of lines the array can hold.

The problem is it doesnt work.

What do you mean by check the return type? I want it to return a pointer to a character string.

I know there will be some wastage if the data is less than BUFFESIZE.

I know there is a problem with my code I have been racking my brain over it for the last two hours trying to find out which line is causing it to read or write outside of the allocated memory... but I am completely lost. The debugger I got is rubbish.

What i meant is this line

strcpy(pa+(n*(BUFFESIZE * sizeof(char))),buffer);

suppose buffer read 80 characters. and n =0 in the first loop. At that time pa is allocated with '5' (line max) . Then you are copying 80 characters to a buffer of size 5.
The same way in all the loops. pa does nt seem to have enough size to hold buffer

[N1GHTS]

@Dazaa

// ...

#define BUFFESIZE 80

// ...

int linemax = 5;
char *pa = (char *)malloc(sizeof(char) * linemax); 

// Now pa has 5 bytes of memory

char buffer[BUFFESIZE];

// Now buffer has 80 bytes of memory

// ...

int n = 0;

// ...

if (n < linemax) {

// n is 0, so it goes in...

fgets(buffer,BUFFESIZE,f);

// Copies the string to buffer, which has 80 bytes in it

strcpy(pa+(n*(BUFFESIZE * sizeof(char))),buffer);  // <---

// Here is what it looks like simplified:

strcpy(pa + (0 * (80 * 1)), buffer);

// simplified further...

strcpy(pa, buffer);

// Remember how pa had 5 bytes of memory? How will buffer = 80 fit inside pa = 5?

In other words, realloc() was not the problem, it was your logic.

@sree_ec

if you cast malloc like this:

i = (int*) malloc(10);

You are creating an array of ints. Problem is, 10 doesn't go into 4 nicely, so your third element in the int array is going to give you a runtime error... if your lucky.

The propper use of what you just did there is this...

i = (int*) malloc(3 * sizeof(int));

You can cast malloc to anything.. If the space you allocated is BIGGER than your cast, you automatically created an array. If its smaller, then you either made a mistake or you just need to pointer and don't care about how its being stored... for instance:

char *Buf;

Buf = (char *)malloc(sizeof(some_struct));

// Later...

((some_struct *)Buf)->Variable = 2;

[Dazaa]

aww u legend.
It wasnt just that line either.

I had not added it to realloc too.

wrong lines were:

char *pa = (char *)malloc(sizeof(char) * BUFFESIZE * linemax);

AND

char *pa = (char *)malloc(sizeof(char) * BUFFESIZE * linemax);

[sree_ec]

aww u legend.
It wasnt just that line either.

I had not added it to realloc too.

wrong lines were:

char *pa = (char *)malloc(sizeof(char) * BUFFESIZE * linemax);

AND

char *pa = (char *)malloc(sizeof(char) * BUFFESIZE * linemax);

Expecting that you meant malloc and realloc...

This is exactly what I meant by Asking you
"Why you are reallocating with respect to linemax when you are copying Atleast BUFFSIZE to pa in each strcpy" in my first post ... :)