why 50% of allocated memory is not freed?

[Ben Bacarisse]

Ben Bacarisse <[email protected]> writes:

I would write the switch as an if because there are really only two
cases:

if (i < 2)
table = i;
else table = table[i-1] * 2;

and I would really want to write:

table = i < 2 ? i : table[i-1] * 2;

but coding style in force might prevent me. I like conditional
expressions because they often seem to make more explicit what the
pattern really is but they seem to be out of favour with bosses.

Which is strange because your use of "?:" is identical and if someone
can not read that as C then, IMO, they have no place modifying the
code.

Some bosses object to such code because it is not "debugger
friendly". I have seen people complain about forms like

return *error = NO_SPACE, NULL;

on similar grounds.

 

[Kaz Kylheku]

MN wrote:

(I've no idea why the newsreplier has trashed your spaces)

table = malloc (sizeof(int*)* max);

for (i = 0; i < max; i++)
{
if (i == 0)
table = 0;
else
{
if (i == 1)
table= 1;
else
table = table[i-1]*2;
}
}

The initialisation code seems unnecessarily complicated.

table[0] = 0, table[1] = 1;

Careful; you cannot initialize table[max].

 

[CBFalconer]

.... snip ...

Some bosses object to such code because it is not "debugger
friendly". I have seen people complain about forms like

return *error = NO_SPACE, NULL;

on similar grounds.

I think debuggery has nothing to do with it. That code is hostile
to reading.

 

[Barry Schwarz]

On Wed, 25 Feb 2009 04:25:46 -0800 (PST), MN

snip

Hi again,
Thanks for your explanation. I decided to use solution a).
I reworte my code and even addanother function, which call to
"test_foo" function.

Then there was really no reason to quote 100+ lines of obsolete code.
Please trim your posts.

Now there is no memory leak in my program. Thanks again to of you.

The new code is:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

const int max_length = 15;
const int s_length = 2;


int* table(int max)
{
int i = 0;
int* table = NULL;

table = malloc(max * sizeof *table);
if (!table)
{
fprintf(stderr, "table is Out of memory\n");
exit (8);

This is not a portable value to return to the operating system. Use
EXIT_FAILURE and others will be able to execute your code.

}

for (i = 0; i < max; i++)
{
switch (i)
{
case 0: table = 0;
break;
case 1:
table = 1;
break;
default:
table = table[i-1] * 2;
break;
}
}
return table;
}

char** test_foo(int max, int s)
{
int i = 0;
char** test = NULL;

test = malloc(max * sizeof(*test));
if (!test)
{
fprintf(stderr, "test is Out of memory\n");
exit (8);

If you reach this code, you have not freed the memory table_tmp points
to. There is no guarantee the system will do it for you, though most
probably do.

}

for (i = 0; i< max; ++i)
{
test = malloc((s+1) * sizeof(*test));
if (!test)
{
fprintf(stderr, "test[%d] is Out of memory\n", i);
exit (8);

If you reach this code, you have not freed any of the memory that
previous test's point to or the memory test points to.

}
}

for (i = 0; i< max; ++i)
sprintf(test, "%d", i);
return test;
}

void test_bar(int max, int s)
{
char** bar = NULL;
char** ptr = NULL;

int k = 0;

bar = malloc(max * sizeof(*bar));
if (!bar)
{
fprintf(stderr, "bar is Out of memory\n");
exit (8);
}

for (k = 0; k < max; ++k)
{
bar[k] = malloc((s+1) * sizeof(*bar[k]));
if (!bar[k])
{
fprintf(stderr, "bar[%d] is Out of memory\n", k);
exit (8);
}
}

ptr = test_foo(max, s);

for (k = 0; k < max; ++k)
{
strcpy(bar[k], ptr[k]);
printf("bar[%d] = %s\n",k, bar[k]);
}
for (k = 0; k < max; ++k)
{
free(ptr[k]);
free(bar[k]);
}
free(ptr);
free(bar);
return ;
}

int main (void)
{
int* table_tmp = NULL;
char** test_tmp = NULL;
int j = 0;

table_tmp = table (max_length);

for (j = 0; j < max_length; ++j)
printf("table_tmp [%d] = %d\n", j, table_tmp[j]);
printf("\n");

test_tmp = test_foo(max_length, s_length);

for (j = 0; j < max_length; ++j)
printf("test_tmp [%d] = %s\n", j, test_tmp[j]);

test_bar(max_length, s_length);

// Now all allocated memory is freed,
for (j = 0; j < max_length; ++j)
free (test_tmp[j]);

free (test_tmp);
free (table_tmp);

return 0;
}

 

[Guest]

Ben Bacarisse wrote:

who cares?

 I have seen people complain about forms like



I think debuggery has nothing to do with it.  That code is hostile
to reading.

agreed, it seems unnecessarily "clever". But then I don't
like the comma operator

 

[Ben Bacarisse]

who cares?

I don't. I was asked why someone /might/ object so I aired the only
objection that I had heard. In fact I thought it was the questioner
(Richard) who had made the objection at one time, but it could well
have been about similar but significantly different code.

agreed, it seems unnecessarily "clever". But then I don't
like the comma operator

I don't use the style myself but, equally, I don't find it impedes
reading. It could be argued that it helps to have a few things that
make you pay attention when reading code -- a bit like adding bends to
motorways.

 

[Richard Bos]

Some bosses object to such code because it is not "debugger
friendly". I have seen people complain about forms like

return *error = NO_SPACE, NULL;

on similar grounds.

I would not object to such code because of any hypothetical debugger,
but because it is not programmer-friendly. There is no earthly reason
not to just write it as

*error=NO_SPACE;
return NULL;

except for its author having made one too many five-point landings off
the bars.

Richard

 

[Mark Wooding]

I would not object to such code because of any hypothetical debugger,
but because it is not programmer-friendly. There is no earthly reason
not to just write it as

*error=NO_SPACE;
return NULL;

That will take more vertical lines. Especially so if the code in
question is guarded by an `if' (as is quite likely): separate statements
will probably want three lines (an extra one for a `}') as opposed to
just one line for Ben's version. My editor only shows me 105 lines of
code in a column on this screen (and it's a fairly big screen -- I don't
know how people cope with these crappy `widescreen' affairs), so
vertical space is precious.

Besides, I think gluing the error-code setting into the return statement
is a fairly good fit. That doesn't mean I'm going to start doing this,
but I'll reserve the right...

-- [mdw]

 

[CBFalconer]

That will take more vertical lines. Especially so if the code in

.... snip ...

No it doesn't. What is wrong with:

*error = NO_SPACE; return NULL;

one more semi, one less comma, than the original. No confusion.

 

[Kaz Kylheku]

just one line for Ben's version. My editor only shows me 105 lines of
code in a column on this screen (and it's a fairly big screen -- I don't
know how people cope with these crappy `widescreen' affairs), so
vertical space is precious.

Some people cope by turning those wide displays 90 degrees for editing
documents or coding.

 

[Richard]

... snip ...

No it doesn't. What is wrong with:

*error = NO_SPACE; return NULL;

one more semi, one less comma, than the original. No confusion.

It would fail any code review that is what is wrong. Multiple statements
like that on a single line are a no no.

 

[Mark Wooding]

... snip ...

No it doesn't. What is wrong with:

*error = NO_SPACE; return NULL;

one more semi, one less comma, than the original. No confusion.

And the extra `}' when (as will inevitably occur) this is guarded by
`if'. Which I did mention, but you snipped:
[...]

I've tried writing stuff like

if ((p = malloc(n)) == 0)
{ *error = NOMEM; return (0); }

but the indented braces rub me up the wrong way: it's too much like
RMS's crazed GNU style.

if ((p = malloc(n)) == 0) {
*error = NOMEM; return (0); }

is just evil. In this particular case, because the `if' guard is short,
I might write

if ((p = malloc(n)) == 0) { *error = NOMEM; return (0); }

the whole thing on one line, but in practice having `n' lying around so
conveniently is unlikely.

Besides, that also loses the logical `I'm sort-of `returning' this error
code to the caller, so it goes in the `return' statement', which, now I
think about it, I also mentioned before:

The others are right. You really do need to learn to read.

(Yes, the extra detail is just to keep my original-text stats high.)

-- [mdw]

 

[Richard Bos]

That will take more vertical lines.

Certainly, but it is an extra line which makes the code more readable
and maintainable. If you _must_ compress for minimum newline count, at
least do it like this:

*error=NO_SPACE; return NULL;

Besides, I think gluing the error-code setting into the return statement
is a fairly good fit.

Only marginally.

Richard