IS this a proper way of freeing memory with free()

[Yevgen Muntyan]

What are you NOT getting about the fact that the cast is not needed in ISO C?
Do you know the purpose and characteristics of void pointers?

You're not kidding, are you? I said *I need* cast because I want a safer
expression. If I apply cast, the expression acquires a type.

((Type*) malloc (sizeof (Type)))

is a standalone expression of type Type* (and it's an allocated chunk
of memory). Of course I can avoid cast here, but I believe void* is
worse than Type* when I need Type*.

I do get the fact that cast is not needed in ISO C.

malloc() takes a size_t, that is IT. It does not talk to, worth with, or have
beers with sizeof to figure out what the type is.

Yep, and this is why I apply cast to make an expression which has beer
with its argument every Friday.

Yevgen

 

[Yevgen Muntyan]

Which is more likely to happen with the second case. You're flip flopping
around here.

Nope. It is not "more likely to happen" if you use real correct type
you need. If you do

struct Child {
Parent p;
...
};

Parent *ptr = malloc (sizeof (Child))

or similarly

Parent *ptr = some_func (sizeof (Child));

where some_func does some job inside, you're fine; but

Parent *ptr = malloc (sizeof *ptr);

is broken. And things like this happen.

You don't feel it's really a style issue, do you.

Yevgen

 

[Christopher Layne]

You're not kidding, are you? I said *I need* cast because I want a safer
expression. If I apply cast, the expression acquires a type.

((Type*) malloc (sizeof (Type)))

Here's a grand concept:

1. Don't use sizeof(Type) to malloc().
2. Don't use the wrong allocation functions within the functional context you
desire.
3. If you need to allocate memory for an aggregate type such as a structure,
do indeed do what you're referring to, and wrap that up in it's own
blah_new() function. However, you still wouldn't cast malloc within that
function either.
4. If you do need a custom allocator, which refers back to 3 above, you won't
be returning the value of ((Type *)malloc(sizeof (Type)) because you actually
have to do something *custom* to the new object in the first place.
5. If you cannot prevent yourself from writing code like the following:

char *q;
q = malloc(sizeof(struct i_want_c_plus_plus));

Then what do you expect?
It's the entire reason we're advocating not writing sizeof (type) in there in
the first place. Now, I know you're going to go back to "custom allocator",
well...

*This* is an example of a real world "custom allocator":

thing *thing_new(void)
{
thing *t;

t = malloc(sizeof *t); /* no sizeof (thing) and no cast */
if (t == NULL) problem();
t->flags = THING_FLAGS_DEFAULT;
t->queue = thing_queue_new();
t->mutex = malloc(sizeof *t->mutex);
mutex_init(t->mutex, NULL);
t->newsreader = 0;
t->stop_posting = 1;

return t;
}

That's actually doing something like initializing lower level entities the
caller doesn't need to know about and encapsulating common tasks to one area.

This, on the other hand...

thing *thing_new(void)
{
return malloc(sizeof (thing)); /* no cast */
}

Is not a custom allocator. If anything it's just a waste of space. But notice
I didn't cast malloc() there either?

 

[Christopher Layne]

Nope. It is not "more likely to happen" if you use real correct type
you need. If you do

struct Child {
Parent p;
...
};

Parent *ptr = malloc (sizeof (Child))

Which of course would actually be wrong based on your example.

or similarly

Parent *ptr = some_func (sizeof (Child));

where some_func does some job inside, you're fine; but

Parent *ptr = malloc (sizeof *ptr);

is broken. And things like this happen.

It's completely not broken.

You don't feel it's really a style issue, do you.

Yevgen

What exactly is your issue? Get your types right in the first place and you
won't have to use a bunch of goofy language tricks to catch straight up bad
programming.

You want C to be C++. Not gonna happen.

 

[Flash Gordon]

Yevgen Muntyan wrote, On 09/02/07 12:47:
I note you failed to address the point about your example not being
applicable.

You also fail to address this point.

Exactly, malloc is typeless. And it's good to wrap blind typeless malloc
into a macro which gets an expression which does have type. I NEED to
cast return value to get an expression with type, because I want that
expression to have type. I NEED an expression which reads "allocate one
Foo structure". I don't like an expression which reads "allocate
sizeof(the-type-of-this-pointer-dereferenced) bytes".

Then write a fooalocator function that returns a pointer to a properly
initialised foo. It *still* does not need a cast.

> I want compiler
to warn me if I assign that Foo* to a variable of wrong type or
return it from a function with different return type, or feed it to
a function with different type of argument.

If the function takes bar* and you pass it a foo* then the compiler
*will* warn you, this has nothing to do with casting the value returned
by malloc. The same for return values.

I guess you're right here to some extent. Or maybe I refuse to do things
in a worse way than I do them now?

No, you refuse to admit to the possibility that what has been suggested
might be better. You seem to ignore at least half the counter points to
your argument. Arguing with someone who does not address the counters to
the points he raises is pointless, so bye.

 

[Yevgen Muntyan]

Here's a grand concept:

1. Don't use sizeof(Type) to malloc().

Because it's wrong, huh?

2. Don't use the wrong allocation functions within the functional context you
desire.

3. If you need to allocate memory for an aggregate type such as a structure,
do indeed do what you're referring to, and wrap that up in it's own
blah_new() function. However, you still wouldn't cast malloc within that
function either.

If I call malloc in code, i.e. if I write in a function body

.... malloc (...);

then I won't cast it. Who said I do?

4. If you do need a custom allocator, which refers back to 3 above, you won't
be returning the value of ((Type *)malloc(sizeof (Type)) because you actually
have to do something *custom* to the new object in the first place.
5. If you cannot prevent yourself from writing code like the following:

char *q;
q = malloc(sizeof(struct i_want_c_plus_plus));

Sorry, I do not write code like that. And if I use nicish macro
with a cast, I can't write such code again because compiler will
tell I am converting wrong stuff to wrong stuff.

Then what do you expect?
It's the entire reason we're advocating not writing sizeof (type) in there in
the first place. Now, I know you're going to go back to "custom allocator",
well...

Nah, custom allocator meant replacement for malloc(), not foo_new()
which does more than allocating memory. Anyway, here you go:

http://svn.gnome.org/viewcvs/glib/trunk/glib/gmem.h?view=markup

Nobody is putting

"(Type*) malloc (sizeof (Type))"

literally in code. But "g_new(Type, 1)" is a nicer expression saying
"allocate one Type structure". Now you may not think it's nicer, but
I do think it is. And I think it's a style issue. You like void*,
I don't. And I am not the person trying to convince the other to do
what he does here. I'm fine with others doing malloc(sizeof *ptr).
I am not fine with others telling me to do that and trying to prove
that that form is better while it's not.

 

[Yevgen Muntyan]

Yevgen Muntyan wrote, On 09/02/07 12:47:

I note you failed to address the point about your example not being
applicable.

ALLOC_A_THING passes default options to
custom_allocator_with_some_options. And yes, I am talking about specific
cases of course. It's said here "ALLOC_A_THING" is never
needed/good/better. I am saying that

ALLOC_A_THING()

may be much better than

alloc_something (sizeof (ThatType), FOO_BAR | FOO_BAZ);

You also fail to address this point.

I didn't want just to say that yes that IS a workaround. You have
to use nasty sizeof *ptr to make sure size if correct there. I believe
another workaround is better: provide correct type and make the result
have that type instead of void*. And "better" here means "nicer for me,
and no worse in any technical aspect".

Then write a fooalocator function that returns a pointer to a properly
initialised foo. It *still* does not need a cast.

Now your argument is "don't do this and do that instead". You may think
it's good to write thousand

AnyStruct *
any_struct_new (void)
{
return malloc (sizeof (AnyStruct));
}

functions, duplicate them in all source files since they must be static
(or make sure their names don't clash with anything else, and then
make sure the symbols are not exported from the lib), I don't think so.
Now, if there is no allocate_foo(), then if I need raw chunk of memory
for struct Foo, I prefer an expression of type Foo* to an expression
of type void*.

If the function takes bar* and you pass it a foo* then the compiler
*will* warn you, this has nothing to do with casting the value returned
by malloc. The same for return values.

How about assignment? Your workaround is to use "sizeof *ptr" thing,
mine is to use an expression of correct type. As to arguments and
return values I was thinking rather of

Something *func (void)
{
return malloc (sizeof (Something));
}

(the fooallocator function). This is a bit silly example of course,
but though it's already better than invocation of malloc right in code;
and I sometimes have such functions to make them do more later.
Yeah yeah, I *must* make it

Something *func (void)
{
Something *s;
s = malloc (sizeof (Something));
return s;
}

to avoid being beaten by typeless malloc.

No, you refuse to admit to the possibility that what has been suggested
might be better. You seem to ignore at least half the counter points to
your argument. Arguing with someone who does not address the counters to
the points he raises is pointless, so bye.

It's hard to argue here and address same points again and again,
and it's hard to express what I think in English, and it's hard
to filter valid questions out of "wrong." junk. See, you are asking
technical questions, and then you make a conclusion about insistance,
refusal, and other words I hardly understand. And then you're saying
I must answer your questions or otherwise it's pointless to talk
to me, but I am stubborn as a donkey in any case, right from start.

Bye.

 

[Christopher Layne]

functions, duplicate them in all source files since they must be static
(or make sure their names don't clash with anything else, and then
make sure the symbols are not exported from the lib), I don't think so.
Now, if there is no allocate_foo(), then if I need raw chunk of memory
for struct Foo, I prefer an expression of type Foo* to an expression
of type void*.

You want C++.

How about assignment? Your workaround is to use "sizeof *ptr" thing,
mine is to use an expression of correct type. As to arguments and
return values I was thinking rather of

Something *func (void)
{
return malloc (sizeof (Something));
}

You want C++.

(the fooallocator function). This is a bit silly example of course,
but though it's already better than invocation of malloc right in code;
and I sometimes have such functions to make them do more later.
Yeah yeah, I *must* make it

Something *func (void)
{
Something *s;
s = malloc (sizeof (Something));
return s;
}

to avoid being beaten by typeless malloc.

So you think the previous function is going to somehow magically return void *
to it's callers? They're both going to return pointer to Something.

You want C++.

It's hard to argue here and address same points again and again,
and it's hard to express what I think in English, and it's hard
to filter valid questions out of "wrong." junk. See, you are asking
technical questions, and then you make a conclusion about insistance,
refusal, and other words I hardly understand. And then you're saying
I must answer your questions or otherwise it's pointless to talk
to me, but I am stubborn as a donkey in any case, right from start.

You're scared of malloc() (which to a *C* programmer is like a hammer to a
carpenter).

You want C++.

 

[Richard Heathfield]

[Others here have responded eloquently to Mr Muntyan's reply to my reply
somewhat upthread, so I'll let that go, and focus on where we've got
to.]

Yevgen Muntyan said:

Now your argument is "don't do this and do that instead". You may
think it's good to write thousand

AnyStruct *
any_struct_new (void)
{
return malloc (sizeof (AnyStruct));
}

functions,

No, that would be daft, because you're not adding any value over malloc
itself, so you might as well just do:

AnyStruct *p = malloc(sizeof *p);

Where the thousand functions score is in initialising objects properly.
For example:

AnyStruct *any_struct_new(int x, int y, double z)
{
AnyStruct blank = {0};
AnyStruct *new = malloc(sizeof *new);
if(new != NULL)
{
*new = blank; /* make all values determinate
(if considered desirable) */
new->x = x;
new->y = y;
new->z = z;
}
return new;
}

duplicate them in all source files since they must be
static (or make sure their names don't clash with anything else, and
then make sure the symbols are not exported from the lib),

I don't think so.

I do.

Now, if there is no allocate_foo(), then if I need raw chunk
of memory for struct Foo, I prefer an expression of type Foo* to an
expression of type void*.

That's your choice, but it doesn't actually buy you any type safety.

<snip>

 

[Yevgen Muntyan]

You want C++.
Nope.


You want C++.

Sure you know better.

So you think the previous function is going to somehow magically return void *
to it's callers? They're both going to return pointer to Something.

Yes. And

Something *func (void)
{
return malloc (sizeof (OopsWrongType));
}

is going to return pointer to Something too.

You want C++.
Sure.


You're scared of malloc() (which to a *C* programmer is like a hammer to a
carpenter).

I am scared of void*. Was beaten by this beast not once.

You want C++.

Absolutely. Doing C++ since now on, right away.

Yevgen

 

[Ian Collins]

Ian Collins said:

Ian Collins said:

Richard Heathfield wrote:

Yevgen Muntyan said:

It's easy to write lot of C code which is valid C++. You have to
apply casts to return value of malloc though.

It's *pointless* to write lots of C code which is valid C++. [...]

It's not pointless if you are using C++ for hardware simulations to
test your C drivers.

If you're using C++ for the hardware simulations, you're using C++,
not C.

The driver code is C.

Fine, in which case it isn't C++, so you don't need the cast on malloc.

I'm not sure if you are being deliberately obtuse.

My point is there is sometimes value in having C code which is valid
C++. The driver code in question is compiled as C for the embedded
target, but compiled with the host C++ compiler for testing in a
hardware simulator.

So the code in question is written in the subset of C that is valid C++.

 

[Richard Heathfield]

Ian Collins said:

I'm not sure if you are being deliberately obtuse.

No, I'm usually pretty acute.

My point is there is sometimes value in having C code which is valid
C++. The driver code in question is compiled as C for the embedded
target, but compiled with the host C++ compiler for testing in a
hardware simulator.

Not wise, in my opinion. Your test environment does not adequately
reflect your production environment. You wouldn't try to pull that
stunt with Perl on the target and COBOL on the simulator, would you? So
why do it with C and C++?

So the code in question is written in the subset of C that is valid
C++.

With identical semantics? Are you *sure*? And once you've answered in
the affirmative, ask yourself whether you are *really* sure.

 

[Ian Collins]

Ian Collins said:


No, I'm usually pretty acute.

I had noticed...

Not wise, in my opinion. Your test environment does not adequately
reflect your production environment.

The value gained form being able to test drivers or a complete embedded
system off the target. We built up a very accurate simulation of the
micro we were using which enabled us to fully test our software before
we had hardware.

You wouldn't try to pull that
stunt with Perl on the target and COBOL on the simulator, would you? So
why do it with C and C++?

Is there a common subset of Perl and COBOL?

With identical semantics? Are you *sure*? And once you've answered in
the affirmative, ask yourself whether you are *really* sure.

Yes, and we back that up with acceptance tests that run against the
simulation and the final product.

 

[Christopher Layne]

I'm not sure if you are being deliberately obtuse.

My point is there is sometimes value in having C code which is valid
C++. The driver code in question is compiled as C for the embedded
target, but compiled with the host C++ compiler for testing in a
hardware simulator.

So the code in question is written in the subset of C that is valid C++.

Why not just compile them both under C mode?

 

[Ian Collins]

Ian Collins wrote:




Why not just compile them both under C mode?

The simulation uses C++ for things like hardware registers. In C mode,
a register is a typedef for the appropriate unsigned integer type. In
C++ mode, it can be a class object with conversion and assignment
operators that trigger state changes in the simulation when the register
is written or read.

 

[Stephen Sprunk]

It's not pointless if you are using C++ for hardware simulations to
test
your C drivers.

But you're _not_ testing your C drivers, you're testing
similar-but-not-identical C++ drivers.

Even if your code uses the common subset syntax-wise, you may
accidentally use constructs that mean different (but equally valid)
things in the two languages, e.g. const, or run into compiler
optimization differences.

C++ provides a simple and convenient mechanism for mixing in C objects.
Why not use it and avoid the potential pitfalls?

S

 

[Keith Thompson]

You're not kidding, are you? I said *I need* cast because I want a safer
expression. If I apply cast, the expression acquires a type.

((Type*) malloc (sizeof (Type)))

Ok, I can *sort of* see your point here. Without the cast, the
expression is of type void*, which will be implicitly converted to any
pointer-to-object type. It's less safe in the sense that if you apply
sizeof to the wrong thing, the compiler won't warn you about it:

(A):
double *p1;
int *p2;
p1 = malloc(sizeof *p2);

Whereas if you use the type, then getting the type wrong will give you
a constraint error and a required diagnostic:

(B):
double *p1;
int *p2;
p1 = (int*)malloc(sizeof(int)); /* Compiler catches error */

But if you have the correct type in the cast, but the wrong type in
the sizeof argument, you're back to another error that the compiler
won't catch:

(C):
double *p1;
int *p2;
p1 = (double*)malloc(sizeof(int));

What you're doing is basically case (B) with the two occurrences of
the type name kept in synch by using a macro.

But if you want to use a macro, why not apply the same technique to
case (A), giving you just as much safety?

(D):
#define NEW_OBJ(ptr) ((ptr) = malloc(sizeof *(ptr)))
#define NEW_ARR(ptr, count) ((ptr) = malloc(count * sizeof *(ptr)))

double *p1;
int *p2;
NEW_OBJ(p1);
if (p1 == NULL) {
/* allocation failed */
}
NEW_ARR(p2, 10);
if (p2 == NULL) {
/* allocation failed */
}

Personally, I'm content to keep the sizeof argument consistent
manually, but you can use macros to do this for you *without* using
unnecessary casts.

 

[Stephen Sprunk]

Indeed, code tends to break when you don't touch it. You
put cast in, it explodes in two months.

Bit rot is a very real phenomenon, though Murphy's Law states that your
code won't explode until you're doing a demo for an important customer.

What you're all saying is why it's not a good idea to
use cast. And that's indeed true, it's not a good idea
to cast when you don't have to. What's not true
is that any given piece of code with cast is wrong and
dangerous in any situation.

It's not guaranteed to be wrong. However, casting hides the cases where
it _is_ wrong, and not casting exposes that. Coding so that you expose
bugs, not hide them, is good practice.

S

 

[Ian Collins]

But you're _not_ testing your C drivers, you're testing
similar-but-not-identical C++ drivers.

You could argue the same for any off-target testing. The generated code
is different. The logic remains the same.

Even if your code uses the common subset syntax-wise, you may
accidentally use constructs that mean different (but equally valid)
things in the two languages, e.g. const, or run into compiler
optimization differences.

I've been using this technique for fifteen years or so, using the same
evolving set of tools, so I know the differences between the two and how
to avoid them. I've even checked the assembler output for various bits
of code compiled with the C and C++ compilers.

The advantages of developing and debugging the drivers on a host rather
then a target environment (less these days than a decade ago) still
outweighs any risks. As I said elsethread, I always use automated
acceptance tests that run on both the target and the simulation. Host
based simulations aren't a replacement for target testing, but they can
be a replacement for target debugging.

C++ provides a simple and convenient mechanism for mixing in C objects.
Why not use it and avoid the potential pitfalls?

The code under test is C and I abhor conditional compilation for testing
in source files.

 

[Keith Thompson]

My point is there is sometimes value in having C code which is valid
C++. The driver code in question is compiled as C for the embedded
target, but compiled with the host C++ compiler for testing in a
hardware simulator.

So the code in question is written in the subset of C that is valid C++.

Interesting.

You need your drivers to be callable from C++ on the host system, so
you write them to be compilable as either C or C++, restricting
yourself to the intersection of the two languages. It seems to me
your requirement is perfectly reasonable, but I'm not convinced you've
found the best solution. Have you considered instead compiling your
drivers as C on the host system, and calling them from C++ using C++'s
'extern "C"' feature? Your headers would still have to be valid as
both C and C++ (possibly using "#ifdef __cplusplus" here and there),
but the actual implementation of the drivers could be pure C.