Help with warning when using calloc/malloc

M

MK

I am a newbie. Please help. The following warning is issued by gcc-3.2.2
compiler (pc Linux):
==================================================================
read_raw_data.c:51: warning: assignment makes pointer from integer
without a cast
==================================================================

when the following piece of code was compiled. The offending statement
is calloc. A similar statement in the main() function passes without any
warning. Am I allowed to use calloc (and malloc) only within main() or
am I doing something very silly ??
=============================================
init(char *file, double *grid, \
double *start){


double *depth;
int pts
depth = calloc(pts,sizeof (double));

}
============================================

Thanks.

MK
 
T

Tom St Denis

MK said:
I am a newbie. Please help. The following warning is issued by gcc-3.2.2
compiler (pc Linux):
==================================================================
read_raw_data.c:51: warning: assignment makes pointer from integer
without a cast
==================================================================

when the following piece of code was compiled. The offending statement
is calloc. A similar statement in the main() function passes without any
warning. Am I allowed to use calloc (and malloc) only within main() or
am I doing something very silly ??
=============================================
init(char *file, double *grid, \
double *start){


double *depth;
int pts
depth = calloc(pts,sizeof (double));

}

Did you include stdlib.h?

Tom
 
R

Ricky Cormier

MK,

1. You are not initialising pts with a value
2. You should cast the (void *) returned from calloc to (double *)
3. When declaring pointers it's a good idea to initialise them to NULL to
start with (memory functions like free() will ignore NULL's)

Try the following instead...

init(char *file, double *grid, double *start) {
double *depth = NULL; /* It's always a good idea to initialise ALL
pointers to NULL */
int pts = 5; /* We need ptr to be initialise with a value */
depth = (double *) calloc(pts,sizeof (double)); /* We really should cast
the result to a (double *) */
}

I haven't tried this code on GCC but it compiles fine in VS2003.

Good luck.
 
T

Tom St Denis

Ricky Cormier said:
MK,

1. You are not initialising pts with a value
2. You should cast the (void *) returned from calloc to (double *)

No, no no no no. That hides bugs and is just not required.
3. When declaring pointers it's a good idea to initialise them to NULL to
start with (memory functions like free() will ignore NULL's)

This is retarded. First off, on error they will return NULL anyways.

Second where are my taquitos!

Tom
 
M

Menno Duursma

I am a newbie. Please help. The following warning is issued by gcc-3.2.2
compiler (pc Linux):
==================================================================
read_raw_data.c:51: warning: assignment makes pointer from integer
without a cast
==================================================================

when the following piece of code was compiled. The offending statement
is calloc. A similar statement in the main() function passes without any
warning. Am I allowed to use calloc (and malloc) only within main() or
am I doing something very silly ??

There seem to be some errors, i'll put ^^^ under them.
=============================================
init(char *file,
^^^^
You probably mean "int". But you might want "void" instead (as this
function seems to not return anything, otherwise do return an int).
double *grid, \
^^^
This seems like an escape from the newline - such as seen in Unix
shell-script, no need for that in C, here "carriage return" is just
whitespace like tab or space are.
double *start){


double *depth;
int pts
^^^
Forgot the ";" there...
depth = calloc(pts,sizeof (double));

}
============================================

Thanks.

The rest seems fine. Exept that it takes some arguments, which are unused.
However this snipped may only be "part of" the actual function?
 
R

Richard Bos

[ Do not top-post! ]
1. You are not initialising pts with a value

Not only that, but there's no semicolon on that line.
2. You should cast the (void *) returned from calloc to (double *)

*No* *you* *should* *not*!

This comes up every week or so. malloc() and related functions should
_not_ need the cast in well-written C code. I'm not even going to
enumerate all the reasons why not, ask Google Groups if you want to gain
some wisdom.

What you _should_ do, however, is include the proper header for every
function you use. In this case,

#include <stdlib.h>

Also, using sizeof *pointer instead of sizeof(fixedtype) in allocation
calls makes for more solid code.
3. When declaring pointers it's a good idea to initialise them to NULL to
start with (memory functions like free() will ignore NULL's)

Rarely. Do you initialise all your integers to 0 before assigning a
different value to them? No? Then why treat pointers differently?
Try the following instead...

init(char *file, double *grid, double *start) {
double *depth = NULL; /* It's always a good idea to initialise ALL
pointers to NULL */
int pts = 5; /* We need ptr to be initialise with a value */
depth = (double *) calloc(pts,sizeof (double)); /* We really should cast
the result to a (double *) */
}

Or rather, do this:

/* Put this line somewhere before you first use calloc(),
typically at the start of the file: */
#include <stdlib.h>

/* Implicit int went out with C99, and was never a good idea to begin
with. Use an explicit return type, or void. */
int init(char *file, double *grid, double *start)
{
double *depth;
int pts;

/* Place code here that gives pts a value, or at a pinch, but rather
not, initialise it with a fixed value using, e.g., int pts=10; */
depth = calloc(pts, sizeof *depth);

/* Use depth for something. */

/* And don't forget, somewhere where depth is still is scope, to: */
free(depth);

return some_value;
}
I haven't tried this code on GCC but it compiles fine in VS2003.

Code that works by accident is dangerous code. Hitch up the optimisation
level and it _might_ just crash on you.

Richard
 
R

Richard Bos

Menno Duursma said:
^^^^
You probably mean "int".

Erm... then what's the function called?
^^^
This seems like an escape from the newline - such as seen in Unix
shell-script, no need for that in C, here "carriage return" is just
whitespace like tab or space are.

No need, true, but perfectly legal C. In fact, in some cases it _is_
necessary - AFAIK, since adjacent string literals started to
concatenate, it's only been needed for multi-line macro definitions, but
in that case, it's the only real solution.
The rest seems fine. Exept that it takes some arguments, which are unused.

And that the actual problem is elsewhere.

Richard
 
E

Eric Sosman

Ricky said:
[a top-post, here corrected]

MK said:
I am a newbie. Please help. The following warning is issued by gcc-3.2.2
compiler (pc Linux):
==================================================================
read_raw_data.c:51: warning: assignment makes pointer from integer
without a cast
==================================================================

when the following piece of code was compiled. The offending statement
is calloc. A similar statement in the main() function passes without any
warning. Am I allowed to use calloc (and malloc) only within main() or
am I doing something very silly ??
=============================================
init(char *file, double *grid, \
double *start){


double *depth;
int pts
depth = calloc(pts,sizeof (double));

}
[...]

1. You are not initialising pts with a value

Irrelevant, since `pts' is not being used. The lack
of a trailing semicolon, however, would be troublesome ...
2. You should cast the (void *) returned from calloc to (double *)

No, NO, NO-NO-NO! Had he followed this bad advice, he
would probably not have received the error message. Silencing
the message without curing the error is a poor idea. It's the
appearance of the message that prompted MK to consult this forum;
if he had not been warned of a problem he'd most likely never
have learned how to correct it.
3. When declaring pointers it's a good idea to initialise them to NULL to
start with (memory functions like free() will ignore NULL's)

The parenthetical remark is correct, but the rest of the
sentence is wrong-headed or even foolhardy. One important
disadvantage of the practice is that it prevents the compiler
from diagnosing the use of an uninitialized variable (if the
compiler has that ability). For example,

char *ptr;
switch (state) {
case 0:
ptr = "California";
break;
case 1:
ptr = "Siberia";
break;
}
printf ("You have been exiled to %s\n", ptr);

A helpful compiler will warn you that `ptr' may not have
been initialized by the time the printf() call is executed,
because no assignment will occur if `state' has a value
like, say, 42. A careful programmer will pay heed to the
warning and examine what possible values `state' may hold,
and might perhaps decide to use something other than a
`switch' to inspect the value.

However, if you wrote `char *ptr = NULL;' as recommended
by Mr. Cormier, the compiler would not detect the problem:
`ptr' has been initialized, albeit with a useless value.
Then if `state' is 42, you'll wind up executing

printf ("You have been exiled to %s\n", (char*)NULL);

Every software maintenance study I've ever read has found
that the cost of a bug rises with the amount of time it lives
in the code. Hence, earlier detection leads to lower cost,
so it's to your advantage *not* to suppress helpful warnings.
 
M

Menno Duursma

Erm... then what's the function called?

Oops. Your right, thanks. What i ment here, somthing like:

int some_func( ... )
{
...
return some_int;
}

[ Snip. ]
And that the actual problem is elsewhere.

Well, i'm surprised calling calloc() from main() _did_ work for the OP,
without haveing included stdlib.h ... Using "-Wall" with gcc 3.2.3 i get:

calloc_test.c: In function `main':
calloc_test.c:6: warning: implicit declaration of function `calloc'

But maybe i shouldn't have assumed any compiler will do that ...
 
D

Default User

Menno said:
Well, i'm surprised calling calloc() from main() _did_ work for the OP,
without haveing included stdlib.h ... Using "-Wall" with gcc 3.2.3 i get:

calloc_test.c: In function `main':
calloc_test.c:6: warning: implicit declaration of function `calloc'

That's a feature offered by some compilers, but it is not a required
diagostic.
But maybe i shouldn't have assumed any compiler will do that ...

Wow, you are already more clueful than E.R. Tisdale. Excellent!




Brian Rodenborn
 
E

EvilRix

Tom St Denis said:
No, no no no no. That hides bugs and is just not required.

It is good practice (read elsewhere in this group for the various arguments
made by others far more qualified than me) to do this. Notice I said
'should' and not 'must'! Just out of interest, what bugs will it hide? Good
programming practice suggests it is always a good idea to be explicit when
coding rather than implicit (such as in this case an explicit cast rather
than an implicit one).
This is retarded. First off, on error they will return NULL anyways.

I'm sorry you feel this is retarded. However, it is a good idea to
initialise ANY variable - including pointers! In the case of pointers it's
common practice to use NULL. By initialising the pointer to NULL we have a
guarenteed value to test for. I accept your premise that c/malloc will
return NULL if the call fails (I never claimed otherwise) but what if the
programmer was daft enough to forget to call c/malloc (it could happen) and
we are talking about 5,000 lines of code to trace not 5? If the storage is
initialise to NULL, even if c/malloc wern't called our tests for sucessful
allocation would pick up and flag a problem.
 
E

EvilRix

I'd suggest that the lack of stdlib may have been the reason the original
problem, but the code as it stands is not good and will most liley cause
other problems. See my post further down for clarification.
 
E

EvilRix

Martin Dickopp said:
EvilRix said:
Tom St Denis said:
MK,

1. You are not initialising pts with a value
2. You should cast the (void *) returned from calloc to (double *)

No, no no no no. That hides bugs and is just not required.

[...] Just out of interest, what bugs will it hide? [...]

Um, ok. Not sure I agree since the OP has posted elsewhere the lack of
stdlib (apprently) was his problem - would this really have hidden this?
 
M

Martin Dickopp

EvilRix said:
Martin Dickopp said:
EvilRix said:
MK,

1. You are not initialising pts with a value
2. You should cast the (void *) returned from calloc to (double *)

No, no no no no. That hides bugs and is just not required.

[...] Just out of interest, what bugs will it hide? [...]

The bug the OP had in his code.

Um, ok. Not sure I agree since the OP has posted elsewhere the lack of
stdlib (apprently) was his problem - would this really have hidden this?

Yes. Try it. :)

Without a declaration, the compiler assumes that `calloc' returns `int'.
Usually it emits a warning if you try to assign an `int' to a pointer
type. But with the cast, you tell the compiler "shut up, I know what I'm
doing." Of course, in this case, you /don't/ know what you're doing,
i.e. in the absence of a `calloc' declaration, the code is wrong with or
without cast.

Martin
 
E

EvilRix

Eric Sosman said:
Ricky said:
[a top-post, here corrected]

MK said:
I am a newbie. Please help. The following warning is issued by gcc-3.2.2
compiler (pc Linux):
==================================================================
read_raw_data.c:51: warning: assignment makes pointer from integer
without a cast
==================================================================

when the following piece of code was compiled. The offending statement
is calloc. A similar statement in the main() function passes without any
warning. Am I allowed to use calloc (and malloc) only within main() or
am I doing something very silly ??
=============================================
init(char *file, double *grid, \
double *start){


double *depth;
int pts
depth = calloc(pts,sizeof (double));

}
[...]

1. You are not initialising pts with a value

Irrelevant, since `pts' is not being used. The lack
of a trailing semicolon, however, would be troublesome ...

It's used in the calloc call and unless it is initialise with a value it is
going to return an undefined number of doubles (or am I missing something
here?) since pts has no defined value prior to the call.
No, NO, NO-NO-NO! Had he followed this bad advice, he
would probably not have received the error message. Silencing
the message without curing the error is a poor idea. It's the
appearance of the message that prompted MK to consult this forum;
if he had not been warned of a problem he'd most likely never
have learned how to correct it.

I don't agree (as I'm sure would many others), but fine. I'd suggest MK woul
d do well to read the plethora of posts on the subject of casting returns
from malloc and make his or her own mind up.
The parenthetical remark is correct, but the rest of the
sentence is wrong-headed or even foolhardy. One important
disadvantage of the practice is that it prevents the compiler
from diagnosing the use of an uninitialized variable (if the
compiler has that ability). For example,

char *ptr;
switch (state) {
case 0:
ptr = "California";
break;
case 1:
ptr = "Siberia";
break;
}
printf ("You have been exiled to %s\n", ptr);

A helpful compiler will warn you that `ptr' may not have
been initialized by the time the printf() call is executed,
because no assignment will occur if `state' has a value
like, say, 42. A careful programmer will pay heed to the
warning and examine what possible values `state' may hold,
and might perhaps decide to use something other than a
`switch' to inspect the value.

However, if you wrote `char *ptr = NULL;' as recommended
by Mr. Cormier, the compiler would not detect the problem:
`ptr' has been initialized, albeit with a useless value.
Then if `state' is 42, you'll wind up executing

printf ("You have been exiled to %s\n", (char*)NULL);

Every software maintenance study I've ever read has found
that the cost of a bug rises with the amount of time it lives
in the code. Hence, earlier detection leads to lower cost,
so it's to your advantage *not* to suppress helpful warnings.

I said, 'good idea', however, I accept there are always exceptions to rules.
Regarding you eample, what happens is state is neither 0 nor 1 AND your
compiler isn't feeling very helpful? There is no guarentee the compiler
would pick up your potential bug as it would depend on the compiler and the
warning levels set.

Again, I suggest MK needs to review the many offerings in this group (and
this thread) and decide what suits his/her needs best.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,767
Messages
2,569,570
Members
45,045
Latest member
DRCM

Latest Threads

Top