Help with warning when using calloc/malloc

E

EvilRix

Martin Dickopp said:
EvilRix said:
Martin Dickopp 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. :)

I did and yes you're right. Thanks.
 
R

Richard Bos

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

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.

Worse than that. Passing an invalid value to a pointer is legally
allowed to crash the program, because it invokes undefined behaviour.
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.

Indeed. Post a single good reason for the cast.
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.

Let's put it this way: with the null initialisation, your compiler is
not likely to warn no matter what, and the example still invokes
undefined behaviour; without the initialisation, the error is the very
same, but your compiler _could_ warn you. No guarantees here, except the
guarantee that the initialisation buggers your chances of the warning.

Richard
 
T

Tom St Denis

EvilRix said:
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).

Well in this case it hides the fact that he didn't include stdlib. Also let
the warnings speak for you. Get a good compiler [re: gcc] and crank up the
warnings. Instead of randomly casting things (which can hide other bugs
like overflows and such) actually work to solve the problems found.
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

First off, not at the declaration and second, no it isn't. Again GCC will
warn you about using before declaring. So instead of writing messy code
like

int a=5,c = 3, d= 4;
char aa = 'b', *b = &aa;

etc....

Just declare your variables and initialize them prior to use in the C code.
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.

Then the compiler will warn about using an unitialized value. You do check
your compiler warnings right?

Tom
 
K

Kenneth Brody

(Top-posting corrected.)
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
================================================================== [...]
depth = calloc(pts,sizeof (double));
[...]
Did you include stdlib.h?
No I didn't and that was the problem.
[...]

A perfect example of the perennial "why is it a bad thing to cast the
return value from malloc/calloc" question.

Especially on a system where sizeof(int)!=sizeof(void*).

--

+---------+----------------------------------+-----------------------------+
| Kenneth | kenbrody at spamcop.net | "The opinions expressed |
| J. | http://www.hvcomputer.com | herein are not necessarily |
| Brody | http://www.fptech.com | those of fP Technologies." |
+---------+----------------------------------+-----------------------------+
 
K

Kenneth Brody

EvilRix wrote:
[...]
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?

The lack of stdlib _is_ the bug.

--

+---------+----------------------------------+-----------------------------+
| Kenneth | kenbrody at spamcop.net | "The opinions expressed |
| J. | http://www.hvcomputer.com | herein are not necessarily |
| Brody | http://www.fptech.com | those of fP Technologies." |
+---------+----------------------------------+-----------------------------+
 
M

Michael Wojcik

[re casting malloc/calloc]

It is good practice (read elsewhere in this group for the various arguments
made by others far more qualified than me) to do this.

Perhaps you should read those threads again. There were, as I recall,
a grand total of two qualified advocates of casting malloc/calloc in
the most recent tussle over this issue. One, P. J. Plauger, suggested
that the cast was appropriate in certain situations; this is not one
of them. The other, Sidney Cadot, makes an interesting case, but even
he admits the danger of omitting stdlib.h; he just feels that modern
implementations should flag that anyway, eliminating the need to catch
it by not casting malloc/calloc. He's recommending a different trade-
off, in other words.

How you derive "good practice" from that is beyond me. If you believe,
for some unimaginable reason, that casting malloc/calloc is "good
practice", fine; but vague references to recent threads in c.l.c to
support your claim is bogus.
Notice I said
'should' and not 'must'! Just out of interest, what bugs will it hide?

Other posters have already noted the first one: it hides the failure
to include stdlib.h and have a prototype for calloc in scope.

It also hides the fact that the code invokes undefined behavior,
because it creates a pointer from an int. And yes, there are
conforming hosted C implementations in use where you cannot convert
a pointer to an int and back again successfully. One is being
discussed in another thread right now.
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).

I believe good programming practice suggests it is always better to
be right than to be wrong. This code was wrong, and your "fix" to it
was wrong.

Advocate casting calloc/malloc if you like. Don't have much hope of
converting the c.l.c majority to your position, though, and please
don't claim that we're already there.

--
Michael Wojcik (e-mail address removed)

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
-- David Bonde
 
S

Sidney Cadot

It is good practice (read elsewhere in this group for the various arguments
made by others far more qualified than me) to do this.

I have trouble understanding how you could come to this conclusion,
given that the "generally-anti-cast" crowd probably outnumbers the
"generally-pro-cast" crowd by something like 15-to-1 in this newsgroup.
I've seen people claiming it's an open-and-shut case against casts; and
I've seen people making the case that it's not a black-and-white thing,
and a case can be made for casting (that includes me and Mr. Plauger);
however, I have seen no-one claim it is unqualified, beyond-discussion
good practice to do malloc casting, as you suggest.
[...] 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).

"explicit cast" is a tautology, and "implicit cast" does not exist. The
word you're probably looking for is "conversion". I have had to un-train
myself for the same mistake :)

Best regards,

Sidney
 

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

No members online now.

Forum statistics

Threads
474,139
Messages
2,570,807
Members
47,356
Latest member
Tommyhotly

Latest Threads

Top