Memory corruption on freeing a pointer to pointer

[Ian Collins]

On 08/24/2013 03:05 PM, Sharwan Joram wrote:
[...]
if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

Does it matter? The == operator is symmetric, (X==Y) == (Y==X).

Yes, it matters - because it's intended to protect against the typo
which replaces == with =, by ensuring that it would cause a constraint
violation. The assignment operator is very definitely not symmetric.

The best way to protect against that particular typo is decent unit tests.

 

[Ike Naar]

On 08/24/2013 03:05 PM, Sharwan Joram wrote:
[...]
if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

Does it matter? The == operator is symmetric, (X==Y) == (Y==X).

Yes, it matters - because it's intended to protect against the typo
which replaces == with =, by ensuring that it would cause a constraint
violation. The assignment operator is very definitely not symmetric.

The best way to protect against that particular typo is decent unit tests.

Do not overestimate the value of unit tests.
Let's take, for example, a piece of code that multiplies two
32-bit numbers to obtain a 64-bit result.
A decent test (that covers all cases) would require 2^64 test cases
which is at least impractical.

 

[James Kuyper]

James Kuyper wrote: ....

The best way to protect against that particular typo is decent unit tests.

I greatly prefer catching problems during compilation rather than testing.

 

[Ian Collins]

On 08/26/2013 02:40 AM, Ike Naar wrote:
On 08/24/2013 03:05 PM, Sharwan Joram wrote:
[...]
if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

Does it matter? The == operator is symmetric, (X==Y) == (Y==X).

Yes, it matters - because it's intended to protect against the typo
which replaces == with =, by ensuring that it would cause a constraint
violation. The assignment operator is very definitely not symmetric.

The best way to protect against that particular typo is decent unit tests.

Do not overestimate the value of unit tests.
Let's take, for example, a piece of code that multiplies two
32-bit numbers to obtain a 64-bit result.

What has that got to do with this particular typo?

 

[Ian Collins]

I greatly prefer catching problems during compilation rather than testing.

So do I. For my projects make == make test.

 

[Malcolm McLean]

Not every language. Many don't allow assignment in the middle
of expressions. In Fortran, assignment is a statement, not an
operator.

Every language except brainf**k and ilk uses + for addition, and virtually every one uses =, which is never used for addition.
(OK, go on, post some C++ which overloads the assignment
operator).

So the +/= typo applies to every language. It will always generate
an expression which is wrong, in every language known to the
programmer. It leaps out.

OTOH if(Nstates = 52) is correct Fortran with the obvious meaning.
In C, the logic will break when Texas secedes, but it will
appear to work until then. So the error doesn't leap out.

 

[Keith Thompson]

Yes. Equality is a boolean function operating on two operands,
returning "the operands are equal". It does not matter which
one of the operands is on the left side of the operator.

Fascinating.

You are of course absolutely correct with respect to the semantics
of the "==" operator; nevertheless (as I've already said) I find
(42 == x) annoyingly awkward -- as do many other people. I take
your word for it that you don't, but I honestly find it surprising.

 

[Ike Naar]

James Kuyper wrote:
On 08/26/2013 02:40 AM, Ike Naar wrote:
On 08/24/2013 03:05 PM, Sharwan Joram wrote:
[...]
if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

Does it matter? The == operator is symmetric, (X==Y) == (Y==X).

Yes, it matters - because it's intended to protect against the typo
which replaces == with =, by ensuring that it would cause a constraint
violation. The assignment operator is very definitely not symmetric.

The best way to protect against that particular typo is decent unit tests.

Do not overestimate the value of unit tests.
Let's take, for example, a piece of code that multiplies two
32-bit numbers to obtain a 64-bit result.

What has that got to do with this particular typo?

Everything. If you know the typo in your progam, you
don't need tests to find it, you just fix the typo.
If you don't know there's a typo, the only way to be sure you
find one is to test all possible cases.

 

[Ian Collins]

James Kuyper wrote:
On 08/26/2013 02:40 AM, Ike Naar wrote:
On 08/24/2013 03:05 PM, Sharwan Joram wrote:
[...]
if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

Does it matter? The == operator is symmetric, (X==Y) == (Y==X).

Yes, it matters - because it's intended to protect against the typo
which replaces == with =, by ensuring that it would cause a constraint
violation. The assignment operator is very definitely not symmetric.

The best way to protect against that particular typo is decent unit tests.

Do not overestimate the value of unit tests.
Let's take, for example, a piece of code that multiplies two
32-bit numbers to obtain a 64-bit result.

What has that got to do with this particular typo?

Everything. If you know the typo in your progam, you
don't need tests to find it, you just fix the typo.
If you don't know there's a typo, the only way to be sure you
find one is to test all possible cases.

But you will know as soon as you add the code and it fails its test.

 

[Ike Naar]

Fascinating.

You are of course absolutely correct with respect to the semantics
of the "==" operator; nevertheless (as I've already said) I find
(42 == x) annoyingly awkward -- as do many other people. I take
your word for it that you don't, but I honestly find it surprising.

Do you find (42 + x) more or less awkward than (x + 42), and why?
If you don't care, then what's the fundamental difference between
the + operator and the == operator?

 

[Ike Naar]

Ike Naar wrote:
James Kuyper wrote:
On 08/26/2013 02:40 AM, Ike Naar wrote:
On 08/24/2013 03:05 PM, Sharwan Joram wrote:
[...]
if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

Does it matter? The == operator is symmetric, (X==Y) == (Y==X).

Yes, it matters - because it's intended to protect against the typo
which replaces == with =, by ensuring that it would cause a constraint
violation. The assignment operator is very definitely not symmetric.

The best way to protect against that particular typo is decent unit tests.

Do not overestimate the value of unit tests.
Let's take, for example, a piece of code that multiplies two
32-bit numbers to obtain a 64-bit result.

What has that got to do with this particular typo?

Everything. If you know the typo in your progam, you
don't need tests to find it, you just fix the typo.
If you don't know there's a typo, the only way to be sure you
find one is to test all possible cases.

But you will know as soon as you add the code and it fails its test.

That depends. If you're lucky, bad code will fail the tests. If
you're not lucky (i.e. if the test subset did not cover all possible
failures) bad code may pass the tests. As long as the tests don't
cover all possible failures, passing the tests does not guarantee
the code is okay.

 

[gdotone]

(e-mail address removed) writes:

<snip>

char **a;
int numberOfPointers = 5;


a = (char **) malloc ( numberOfPointers * ( sizeof(char * ) ) );
if ( a!= NULL )
printf( "the array, a, points to declare space\n\n" );

char *b[] = { "one", "two", "three", "four", "five" };

a = b;

It's possible that you don't know what this assignment does. It only
replaces one pointer with another. The pointer value that used to be in
'a' is lost so you can no longer access the memory that was allocated.
in particular, you can't ever free it now.
<snip>

Ben, so a=b; , does not assign a to point to whatever b points to?
it replaces?
I thought it was having a point to the same memory location.
How would you do that? How would you have a point to b? what is the syntax?

thanks.

 

[glen herrmannsfeldt]

(snip, someone wrote)

Do you find (42 + x) more or less awkward than (x + 42), and why?
If you don't care, then what's the fundamental difference between
the + operator and the == operator?

Algebra commonly puts the coefficient before the variable,
so 2x and not x2. Polynomials are commonly written in order
of decreasing power of x, so x+42 instead if 42+x.

-- glen

 

[James Kuyper]

(e-mail address removed) writes:

<snip>

char **a;
int numberOfPointers = 5;


a = (char **) malloc ( numberOfPointers * ( sizeof(char * ) ) );
if ( a!= NULL )
printf( "the array, a, points to declare space\n\n" );

char *b[] = { "one", "two", "three", "four", "five" };

a = b;

It's possible that you don't know what this assignment does. It only
replaces one pointer with another. The pointer value that used to be in
'a' is lost so you can no longer access the memory that was allocated.
in particular, you can't ever free it now.
<snip>

Ben, so a=b; , does not assign a to point to whatever b points to?

C has special many special rules for arrays, like b. One of those rules
says that, in most context, an lvalue of array type is automatically
converted into a pointer to the first element of that array. This is one
of those contexts. There are three exceptions to that rule:

char *(*c)[5] = &b;

'b' does not get converted into a pointer to it's first element; instead
it remains an array, so &b has the type char*(*)[5].

The second exception is
size_t array_size = sizeof b;
which gives the size of b, and not the size of a pointer to the first
element of b.

The third exception is:
char array[] = "string literal";
where the string literal is an lvalue of array type, but instead of
being automatically converted into a pointer to char (which would be a
constraint violation in this context), it gets used to initialize the
array, exactly as if I had written:
char array[] = {'s', 't', 'r', 'i', 'n', 'g', ' ',
'l', 'i', 't', 'e', 'r', 'a', 'l', '\0'};

In all other contexts, an lvalue of array type gets automatically
converted into a pointer to the first element of the array. This rule
often leads people into confusing pointers and arrays, but they are
quite different (though closely related) things.

As a result of that conversion, the line "a=b;" causes a to contain a
pointer to the first element of b.

it replaces?

Yes, just like numberOfPointers = 6 would cause the value of 5, which is
currently stored in numberOfPointers, to be replaced with the value of
6. That isn't much of a problem, unless you have some need to know what
the old value was - then it becomes a BIG problem.

In this case, before the line "a=b;", the variable 'a' contained a
pointer to memory that had been allocated by a call to malloc(). After
the assignment, 'a' contained a pointer to the first element of b. There
is no other copy of the pointer that used to be stored in 'a'. That
means you no longer have any way to access the memory that the pointer
pointed at. This also means you have no way of freeing that memory.

I thought it was having a point to the same memory location.
How would you do that? How would you have a point to b? what is the syntax?

If you want a to point at the first element of 'b', then there's
absolutely nothing wrong with using "a=b;". The problem is not about
having 'a' point at 'b'. However, 'a' can only point at one thing, it
can either point at the memory you allocated with a call to malloc(), or
it can point at 'b'; it can't point at both. If you want to point at two
different things, then you need two different pointers.

Of course, in this context, it would be a good question to ask why you
want to point at two different things. It would be an even better
question to ask why you have two different things to point at - your
code makes no use of the memory allocated by that call to malloc(), so
your program has no obvious reason for bothering to allocate that memory
in the first place.

What did you think you were allocating that memory for? Whatever it was,
your code isn't actually doing it.

 

[Keith Thompson]

Do you find (42 + x) more or less awkward than (x + 42), and why?

No. Well, now that I think about it, I think I prefer (x + 42), for
reasons I can't necessarily articulate.

If you don't care, then what's the fundamental difference between
the + operator and the == operator?

The difference is that "==" is much more natural with the constant on
the right, and with "+" the difference is less striking.

I know that doesn't answer your question.

"Fred is 42 years old" vs. "42 years old is Fred" might shed *some*
light on it, but it's not a great example ("is" in that context does not
denote equality).

 

[Keith Thompson]

(e-mail address removed) writes:

<snip>

char **a;
int numberOfPointers = 5;


a = (char **) malloc ( numberOfPointers * ( sizeof(char * ) ) );
if ( a!= NULL )
printf( "the array, a, points to declare space\n\n" );

char *b[] = { "one", "two", "three", "four", "five" };

a = b;

It's possible that you don't know what this assignment does. It only
replaces one pointer with another. The pointer value that used to be in
'a' is lost so you can no longer access the memory that was allocated.
in particular, you can't ever free it now.
<snip>

Ben, so a=b; , does not assign a to point to whatever b points to?
it replaces?
I thought it was having a point to the same memory location.
How would you do that? How would you have a point to b? what is the syntax?

`a = b` copies the value of b into a (and is legal only if a
and b are of the same type, or if b is implicitly convertible to
a's type.). That's true whether a and b are integers, structures,
pointers, or anything else that can be assigned. So if a and b are
pointers, then `a = b` copies the value of b (which is an address,
possibly a null pointer) into the object a. After the assignment,
`a == b`, which means either that a and b are both null pointers
or that they both point to the same thing.

To cause a to point to b:

a = &b;

This is legal only if a is a pointer to some type FOO, and b is of type
FOO.

To copy the value of the object b points to into the object that a
points to:

*a = *b;

After this, `a == b` is not necessarily true, but `*a == *b` is true.

 

[James Kuyper]

So do I. For my projects make == make test.

Building my code takes only seconds per module. A comprehensive test of
one module would take anything from minutes up to days to run,
depending upon what precisely that module does. A test that is not
comprehensive would not be an adequate substitute for good warning
messages from the compiler. A test that is comprehensive requires a lot
more work than writing the code that it is testing.

 

[Les Cargill]

On 08/26/2013 02:40 AM, Ike Naar wrote:
On 08/24/2013 03:05 PM, Sharwan Joram wrote:
[...]
if ( NULL == parameters[parametercount]){

This is what's known as a "Yoda conndition"
<http://en.wikipedia.org/wiki/Yoda_Conditions>. I know that a lot of
programmers like them, and for somewhat valid reasons, but personally I
find them jarring and unnecessary. Personally, I'd write that as:

if (parameters[parametercount] == NULL) {

Does it matter? The == operator is symmetric, (X==Y) == (Y==X).

Yes, it matters - because it's intended to protect against the typo
which replaces == with =, by ensuring that it would cause a constraint
violation. The assignment operator is very definitely not symmetric.

The best way to protect against that particular typo is decent unit tests.

Do not overestimate the value of unit tests.
Let's take, for example, a piece of code that multiplies two
32-bit numbers to obtain a 64-bit result.
A decent test (that covers all cases) would require 2^64 test cases
which is at least impractical.

No point in that - use the symmetries of the situation
to make it faster. Walk a one through both operands, for example,
then add some reinforcement about zero and in
the overflow cases if they're signed.

 

[Les Cargill]

The +/= pair. They're generally on the same key, so

drawpixel(x + 2, y = 2);

is a common typo, and it's irritating that it compiles. However
unlike == / =, the difference applies to every language. Even
if you know C very well, often you don't see == / = mistakes,
whilst +/= leaps out.

So find . -name "*.[ch]" | xargs grep if | grep -v "=="

That doesn't get 'em all. but it's close. And yes, I have these things
on my Windows machines, too.

 

[Ian Collins]

Building my code takes only seconds per module. A comprehensive test of
one module would take anything from minutes up to days to run,
depending upon what precisely that module does.

It sounds like your tests aren't unit tests, more like module acceptance
tests.

A test that is not
comprehensive would not be an adequate substitute for good warning
messages from the compiler.

I didn't say it was, but there are plenty of logic errors a compiler
can't spot that can quickly and easily be caught by unit tests
(especially if the code is written to pass the tests). One test for the
quality of unit tests is whether lint spots anything in the code the
tests missed!

A test that is comprehensive requires a lot
more work than writing the code that it is testing.

I agree and that is why I always advocate more than one level of
testing. One mission (not life) critical product I used to manage had
three layers of testing: unit, acceptance and release. The respective
testing cycles were seconds, hours and days.