0/1 Knapsack problem, what am I doing wrong

[Bubba]

Greetings,

I'm attempting to solve a 0/1 knapsack problem as it follows - there are
n objects O_1, O_2, ..., O_n and a knapsack. Object O_i has weight w_i
and value (price) p_i. Capacity of the knapsack is c units of weight.
Solved by dynamic programming approach, M_i,j is denoted as maximal
value that can be obtained my choosing objects from the set {O_1, O_2,
...., O_i} with the capacity j. When M_i,j is achieved, object O_i is
either put in the knapsack or it is not. If O_i is not put in the
knapsack, then M_i,j = M_i-1,j. If O_i is put in the knapsack,
previously chooses object represents the optimal choice from the set {O_
1, O_2, ...,O_i-1} with the capacity j - w_i.

The algorithm fills the matrix with values M_i,j. The value that is
actually searched for is M_n,c. The matrix is formed with dimensions (n+
1) * (c+1), and the order of computing is row by row (since j changes
faster than i). Complexity is O(nc).

The table that should be formed by this algorithm, for values n = 3, (w_
1, w_2, w_3) = (2,3,4), (p_1, p_2, p_3) = (1,7,8) and c = 6 looks like
this:

\ j 0 1 2 3 4 5 6
i
0 0 0 0 0 0 0 0
1 0 0 1 1 1 1 1
2 0 0 1 7 7 8 8
3 0 0 1 7 8 8 9

That is, maximum value in the knapsack should be M_3,6 = 9.

Unfortunately, the code below says otherwise.

#include <stdio.h>

float M[4][7];
int w[3] = {2,3,4};
float p[3] = {1,7,8};

float zero_one_knapsack (int n, int c)
{
int i, j;
for (i=0; i<=n; i++) M[0] = 0.0;
for (j=0; j<=c; j++) M[0][j] = 0.0;
for (i=1; i<=n; i++)
for (j=1; j<=c; j++)
{
M[j] = M[i-1][j];
if ( j >= w )
if ((M[i-1][j-w] + p) > M[i-1][j]) M[j]
= M[i-1][j-w] + p;
}
return M[n][c];
}

int main (void)
{
printf ("%f\n",zero_one_knapsack (3, 6)); /* prints 8 */
return 0;
}

Any help would be appreciated (including the piece of already good code
that could fit my needs stated before ).

TIA!

 

[Bubba]

Eric Sosman's log on stardate 21 tra 2009

if ( j >= w )

if ( j >= w[i-1] )

if ((M[i-1][j-w] + p) > M[i-1][j]) M[j]

if ((M[i-1][j-w[i-1]] + p[i-1]) > M[i-1][j]) M[j]

= M[i-1][j-w] + p;

= M[i-1][j-w[i-1]] + p[i-1];

Eric, that was flawless! I am speechless. Thank you very very very much
for your fast and accurate reaction!

 

[Bubba]

Eric Sosman's log on stardate 21 tra 2009

/snip

Incredible.

Here's the code now:

#include <stdio.h>

double zero_one_knapsack (int n, int c, double **M, double *w, double *p)
{
int i, j;
for (i=0; i<=n; i++) M[0] = 0;
for (j=0; j<=c; j++) M[0][j] = 0;
for (i=1; i<=n; i++)
for (j=1; j<=c; j++)
{
M[j] = M[i-1][j];
if ( j >= w[i-1] )
if ((M[i-1][j-(int)w[i-1]] + (int)p[i-1]) > M[i-1][j]) M[j]
= M[i-1][j-(int)w[i-1]] + (int)p[i-1];
}
return M[n][c];
}

int main (void)
{
int c, n, i;
printf ("Enter number of objects: ");
scanf ("%d", &n);
printf ("Enter knapsack capacity: ");
scanf ("%d", &c);
double **M = (double **)malloc (n + 1 * sizeof (double *));
for (i = 0 ; i < n + 1; i++)
M = (double *)malloc (c + 1 * sizeof (double));
double *w = (double *)malloc (n * sizeof (double));
double *p = (double *)malloc (n * sizeof (double));
for (i = 0 ; i < n ; i++)
{
printf ("Enter weight of %d. object: ", i+1);
scanf ("%lf", w+i);
}
for (i = 0 ; i < n ; i++)
{
printf ("Enter value (price) of %d. object: ", i+1);
scanf ("%lf", p+i);
}
printf ("%f\n",zero_one_knapsack (n, c, M, w, p));
return 0;
}

Compiling on VS 2008 returns 9.000000 but doing the same thing on gcc
version 4.1.2 (x86, Debian) returns 7.000000 (based on the parameters
in the original post)?

What am I doing wrong now?

 

[Bubba]

Bubba's log on stardate 22 tra 2009

#include <stdio.h>

Sorry, copied the code without #include <stdlib.h>.

 

[Chris McDonald]

double zero_one_knapsack (int n, int c, double **M, double *w, double *p)
{
int i, j;
for (i=0; i<=n; i++) M[0] = 0;
for (j=0; j<=c; j++) M[0][j] = 0;
for (i=1; i<=n; i++)
for (j=1; j<=c; j++)

I'm "troubled" by the indentation in the above.
The 2nd loop is not "inside" the 1st, but indentation suggests otherwise.

I'm unfamiliar with the algorithm but I feel that you initial confusion,
so quickly addressed by Eric is due to the indicies i and j sometimes
starting at 0, and othertimes from 1 (??).

double **M = (double **)malloc (n + 1 * sizeof (double *));

Do you mean
double **M = (double **)malloc ((n + 1) * sizeof (double *));

and

M = (double *)malloc (c + 1 * sizeof (double));

M = (double *)malloc ((c + 1) * sizeof (double));

 

[Bubba]

Chris McDonald's log on stardate 22 tra 2009

I'm "troubled" by the indentation in the above.
The 2nd loop is not "inside" the 1st, but indentation suggests
otherwise.
I'm unfamiliar with the algorithm but I feel that you initial
confusion, so quickly addressed by Eric is due to the indicies i and
j sometimes starting at 0, and othertimes from 1 (??).

That one is OK, the "trouble" part really is only in indentation. It
should have been written like this:

for (i=0; i<=n; i++) M[0] = 0;
for (j=0; j<=c; j++) M[0][j] = 0;
for (i=1; i<=n; i++)
for (j=1; j<=c; j++)

double **M = (double **)malloc (n + 1 * sizeof (double *));

Do you mean
double **M = (double **)malloc ((n + 1) * sizeof (double *));
and

M = (double *)malloc (c + 1 * sizeof (double));

M = (double *)malloc ((c + 1) * sizeof (double));

Naturally! Once again, knitting code at 2 AM was not such great idea after
all...

Still, it puzzles me how on earth VS managed to compute the right result.
Never mind, though, as it works as it should now!

Thank you for your swift intervention!

 

[Keith Thompson]

Chris McDonald's log on stardate 22 tra 2009

I'm "troubled" by the indentation in the above.
The 2nd loop is not "inside" the 1st, but indentation suggests
otherwise.
I'm unfamiliar with the algorithm but I feel that you initial
confusion, so quickly addressed by Eric is due to the indicies i and
j sometimes starting at 0, and othertimes from 1 (??).

That one is OK, the "trouble" part really is only in indentation. It
should have been written like this:

for (i=0; i<=n; i++) M[0] = 0;
for (j=0; j<=c; j++) M[0][j] = 0;
for (i=1; i<=n; i++)
for (j=1; j<=c; j++)

No, the third line should be at the same level as the second:

for (i=0; i<=n; i++) M[0] = 0;
for (j=0; j<=c; j++) M[0][j] = 0;
for (i=1; i<=n; i++)
for (j=1; j<=c; j++)

Or, for even greater clarity IMHO):

for (i=0; i<=n; i++)
M[0] = 0;
for (j=0; j<=c; j++)
M[0][j] = 0;
for (i=1; i<=n; i++)
for (j=1; j<=c; j++)

Actually, I'd write it as:

for (i = 0; i <= n; i++) {
M[0] = 0;
}
for (j = 0; j <= c; j++) {
M[0][j] = 0;
}
for (i = 1; i <= n; i++) {
for (j = 1; j <= c; j++) {
/* ... */
}
}

 

[Bubba]

Keith Thompson's log on stardate 22 tra 2009

/snip

True. As I said, let's blame it on being tired...

Here's the whole code, BTW, as far as I've tested it, it works flawless
and I hope it meets all relevant C programming guidelines now.

#include <stdio.h>
#include <stdlib.h>

double zero_one_knapsack (int n, int c, double **M, double *w, double *p)
{
int i, j;
for (i=0; i<=n; i++)
{
M[0] = 0;
}
for (j=0; j<=c; j++)
{
M[0][j] = 0;
}
for (i=1; i<=n; i++)
{
for (j=1; j<=c; j++)
{
M[j] = M[i-1][j];
if ( j >= w[i-1] )
{
if ((M[i-1][j-(int)w[i-1]] + (int)p[i-1]) > M[i-1][j]) M[j]
= M[i-1][j-(int)w[i-1]] + (int)p[i-1];
}
}
return M[n][c];
}

int main (void)
{
int c, n, i;
printf ("Enter number of objects: ");
scanf ("%d", &n);
printf ("Enter knapsack capacity: ");
scanf ("%d", &c);
double **M = (double **)malloc ((n + 1) * sizeof (double *));
for (i = 0 ; i < n + 1; i++)
M = (double *)malloc ((c + 1) * sizeof (double));
double *w = (double *)malloc (n * sizeof (double));
double *p = (double *)malloc (n * sizeof (double));
if (M == NULL || p == NULL || w == NULL)
{
fprintf (stderr, "Error allocating memory, terminating...");
exit(1);
}
for (i = 0 ; i < n ; i++)
{
printf ("Enter weight of %d. object: ", i+1);
scanf ("%lf", w+i);
}
for (i = 0 ; i < n ; i++)
{
printf ("Enter value (price) of %d. object: ", i+1);
scanf ("%lf", p+i);
}
printf ("%f\n",zero_one_knapsack (n, c, M, w, p));
for (i = 0 ; i < n + 1 ; i++)
free(M);
free(M); free(w); free(p);
return 0;
}


Thanks once again to all three of you for your altruistic aid.

 

[Ben Bacarisse]

Keith Thompson's log on stardate 22 tra 2009

/snip

True. As I said, let's blame it on being tired...

Here's the whole code, BTW, as far as I've tested it, it works flawless
and I hope it meets all relevant C programming guidelines now.

A few comments...

#include <stdio.h>
#include <stdlib.h>

double zero_one_knapsack (int n, int c, double **M, double *w, double *p)
{
int i, j;
for (i=0; i<=n; i++)
{
M[0] = 0;
}
for (j=0; j<=c; j++)
{
M[0][j] = 0;
}
for (i=1; i<=n; i++)
{
for (j=1; j<=c; j++)
{
M[j] = M[i-1][j];
if ( j >= w[i-1] )
{
if ((M[i-1][j-(int)w[i-1]] + (int)p[i-1]) > M[i-1][j]) M[j]
= M[i-1][j-(int)w[i-1]] + (int)p[i-1];
}
}

Missing } here!

return M[n][c];
}

Why is everything double? When w[x] and p[x] are used, they must be
converted to int so the effect is that M only holds integers. Using
only integers would simplify a few things.

int main (void)
{
int c, n, i;
printf ("Enter number of objects: ");
scanf ("%d", &n);

It is usually a good idea to test the scanf returns the correct count
(1 in this case). You could have a function that prompts and then
reads an int. This may see like a lot of work for a few cases, but
programming is cumulative. The program you write tomorrow might
benefit from this function so it is almost always worth writing it
now.

printf ("Enter knapsack capacity: ");
scanf ("%d", &c);
double **M = (double **)malloc ((n + 1) * sizeof (double *));

I'd write:

double **M = malloc((n + 1) * sizeof *M);

for (i = 0 ; i < n + 1; i++)
M = (double *)malloc ((c + 1) * sizeof (double));

Same here (and below). This one looks more complex but the pattern is
the same:

M = malloc ((c + 1) * sizeof *M);

Note that if you do this and decide to switch to int rather than
double, none of the malloc calls need to change.

Small point: you are not being consistent. Some of your one-line
loops have {}s and some don't. I favour not having them, but many
people feel differently. Consistency is probably more important than
which of these options you choose.

double *w = (double *)malloc (n * sizeof (double));
double *p = (double *)malloc (n * sizeof (double));
if (M == NULL || p == NULL || w == NULL)

This worries me. It is possible that one of the row allocations in
the loop above could fail, while those for w and p might succeed. You
need to test inside the loop as well. I'd probably write:

for (i = 0 ; i < n + 1; i++)
if ((M = malloc ((c + 1) * sizeof *M)) == NULL)
break;

and then test that the loop ran to the end:

if (i != n + 1 || M == NULL || p == NULL || w == NULL)

{
fprintf (stderr, "Error allocating memory, terminating...");
exit(1);
}
for (i = 0 ; i < n ; i++)
{
printf ("Enter weight of %d. object: ", i+1);
scanf ("%lf", w+i);
}
for (i = 0 ; i < n ; i++)
{
printf ("Enter value (price) of %d. object: ", i+1);
scanf ("%lf", p+i);
}
printf ("%f\n",zero_one_knapsack (n, c, M, w, p));
for (i = 0 ; i < n + 1 ; i++)
free(M);
free(M); free(w); free(p);
return 0;
}

You might want to look at the C FAQ. 6.16 is all about allocating
multi-dimensional arrays. http://c-faq.com/aryptr/dynmuldimary.html

 

[Bubba]

Ben Bacarisse's log on stardate 22 tra 2009

A few comments...

Thx, I was secretly hoping for something like that...

Missing } here!

Noted. I removed all unnecessary parenthesizes.

Why is everything double? When w[x] and p[x] are used, they must be
converted to int so the effect is that M only holds integers. Using
only integers would simplify a few things.

Indeed, this actually, when I look at the algorithm and the code makes
sense...

It is usually a good idea to test the scanf returns the correct count
(1 in this case). You could have a function that prompts and then
reads an int. This may see like a lot of work for a few cases, but
programming is cumulative. The program you write tomorrow might
benefit from this function so it is almost always worth writing it
now.

Something like this? I could call a function for this one, but just as a
concept, is ti OK?

int s_err;
....
s_err = scanf ("%d", &c);
if (s_err != 1 || s_err == EOF)
{
fprintf (stderr, "Error inputing capacity, terminating...");
exit(2);
}

printf ("Enter knapsack capacity: ");
scanf ("%d", &c);
double **M = (double **)malloc ((n + 1) * sizeof (double *));

I'd write:

double **M = malloc((n + 1) * sizeof *M);

for (i = 0 ; i < n + 1; i++)
M = (double *)malloc ((c + 1) * sizeof (double));

Same here (and below). This one looks more complex but the pattern
is the same:

M = malloc ((c + 1) * sizeof *M);

I did as you suggested, but can you clarify the reasons for such
intervention?[1]

Note that if you do this and decide to switch to int rather than
double, none of the malloc calls need to change.

I did switch to int and removed the casts.

Small point: you are not being consistent. Some of your one-line
loops have {}s and some don't. I favour not having them, but many
people feel differently. Consistency is probably more important than
which of these options you choose.

Critics noted. I agree and also dislike {} on oneliners and am naturally
sloppy...

double *w = (double *)malloc (n * sizeof (double));
double *p = (double *)malloc (n * sizeof (double));
if (M == NULL || p == NULL || w == NULL)

This worries me. It is possible that one of the row allocations in
the loop above could fail, while those for w and p might succeed.
You need to test inside the loop as well. I'd probably write:

for (i = 0 ; i < n + 1; i++)
if ((M = malloc ((c + 1) * sizeof *M)) == NULL)
break;

and then test that the loop ran to the end:

if (i != n + 1 || M == NULL || p == NULL || w == NULL)

Thx, yes, I forgot to check the rows allocation...

You might want to look at the C FAQ. 6.16 is all about allocating
multi-dimensional arrays. http://c-faq.com/aryptr/dynmuldimary.html

I did when I wrote the code, that's why I'm perplexed with the
intervention [1].

Thx a lot for your kind comments.

Best regards.

 

[gw7rib]

Actually, I'd write it as:

    for (i = 0; i <= n; i++) {
        M[0] = 0;
    }
    for (j = 0; j <= c; j++) {
        M[0][j] = 0;
    }

Is it just me, or does that look horrible? If you must use brackets,
and if you want to use additional lines (I normally only use the extra
lines if lines are getting too long, which isn't the case here) what's
wrong with:

for (i = 0; i <= n; i++) {
M[0] = 0; }
for (j = 0; j <= c; j++) {
M[0][j] = 0; }

The indenting makes it perfectly clear what is controlled.

Keith Thompson (The_Other_Keith)

And while I'm here - what does this refer to? Which other Keith? And
wouldn't "The other K Thompson" have more cachet?

 

[Keith Thompson]

Actually, I'd write it as:

    for (i = 0; i <= n; i++) {
        M[0] = 0;
    }
    for (j = 0; j <= c; j++) {
        M[0][j] = 0;
    }

Is it just me, or does that look horrible? If you must use brackets,
and if you want to use additional lines (I normally only use the extra
lines if lines are getting too long, which isn't the case here) what's
wrong with:

for (i = 0; i <= n; i++) {
M[0] = 0; }
for (j = 0; j <= c; j++) {
M[0][j] = 0; }

The indenting makes it perfectly clear what is controlled.

It's a matter of taste. The style I use for brace layout is basically
what's used in K&R (except that K&R tends to leave out optional
braces, and I almost always include them).

It's sometimes called the One True Brace Style; see Henry Spencer's
8th Commandment, <http://www.lysator.liu.se/c/ten-commandments.html>.

I'm not sure I have a logical argument against putting the closing
brace at the end of the line, but I personally dislike it, and it's
not a common style. In any case, if you're going to be reading a lot
of C code, you'd better at least get used to seeing the OTBS.

On the other hand, if I'm working on existing code, I'll follow the
conventions established in that code.

And while I'm here - what does this refer to? Which other Keith? And
wouldn't "The other K Thompson" have more cachet?

<EXTREMELY_OT>
When I started my first job out of college, there was another person
at the company named Keith. I've continued using it in my .sig long
after it became irrelevant.
</EXTREMELY_OT>

 

[James Kuyper]

Ben Bacarisse's log on stardate 22 tra 2009 ....

double **M = (double **)malloc ((n + 1) * sizeof (double *));

I'd write:

double **M = malloc((n + 1) * sizeof *M);

for (i = 0 ; i < n + 1; i++)
M = (double *)malloc ((c + 1) * sizeof (double));

Same here (and below). This one looks more complex but the pattern
is the same:

M = malloc ((c + 1) * sizeof *M);

I did as you suggested, but can you clarify the reasons for such
intervention?[1]

sizeof(double) and sizeof *M give the same quantity; so it might seem
that it doesn't matter which one you use. However, if at any future time
the code might need modification, then it does make a difference.

If you change the type of M, sizeof(double) will be incorrect, while
sizeof *M will still be correct.

Of course, there's the flip side of the issue: what if you change the
name of 'M' rather than it's type. In that case, sizeof(double) will
remain the same, whereas sizeof *M will be incorrect.

However, there's two key differences between the two options: if you
change the variable name without changing the sizeof expression, you'll
generally get an error based upon the fact that the old variable name is
no longer defined. Secondly, you can verify whether sizeof *M is
correct just by looking at the left operand of the '=' operator;
verifying whether sizeof(double) is correct will require searching for
the declaration of 'M'. It's certainly possible to do such a search, but
it's a lot more work than verifying.

The second change is removing the cast. This cast is unnecessary,
because it does precisely the same thing that will happen anyway if the
cast were not present. Cases where casts are really necessary are rare,
and are always dangerous; casts should only be used in those situations,
and everytime you see a cast you should automatically think to yourself:
"gomething tricky is going on here, I need to make sure that it's
actually correct". Whenever you use casts where they aren't needed, you
encourage a tendency to ignore casts - and that's bad.

There's a second reason that's specific to C90. If you try to call
malloc() without having a declaration of that function in scope, in C99
it's a constraint violation. However, in C90, in that same situation,
the compiler will create an implicit declaration of malloc() as a
function returning an int. Since malloc() actually returns a void*, this
is an error, though it will accidentally work as intended on some
machines where 'int' and 'void*' have the same size. Without the cast,
you have a constraint violation and the implementation is required to
issue a diagnostic. However, with the cast, it is not a constraint
violation, and no diagnostic is required. In other words, the cast turns
off the diagnostic, and it's a diagnostic you should really want to recieve.

 

[Ben Bacarisse]

Ben Bacarisse's log on stardate 22 tra 2009

Something like this? I could call a function for this one, but just as a
concept, is ti OK?

int s_err;
...
s_err = scanf ("%d", &c);
if (s_err != 1 || s_err == EOF)
{
fprintf (stderr, "Error inputing capacity, terminating...");
exit(2);
}

The two tests are redundant since EOF is negative but, yes, that kind
of thing. Now that everything is integer-based, one function could be
written to prompt for and read a number. You'd use it all over the
place.

I did as you suggested, but can you clarify the reasons for such
intervention?[1]

See James' excellent reply.

You might want to look at the C FAQ. 6.16 is all about allocating
multi-dimensional arrays. http://c-faq.com/aryptr/dynmuldimary.html

I did when I wrote the code, that's why I'm perplexed with the
intervention [1].

You mean you wonder why I suggested one form and the C FAQ uses
another? Who knows? It is small matter of style, but there are
enough things going for the

<exp> = malloc(<size> * sizeof *<exp>);

pattern that I now use it almost always but writing sizeof(<type>) is
not wrong. Note that almost everyone agrees that not casting malloc
is the right thing to do in C.

 

[Barry Schwarz]

James Kuyper wrote:
snip

snip


Not using a cast can also turn off a diagnostic in certain situations.
Those situations aren't any more contrived.

An example please for those of us with limited imaginations.

 

[Ben Bacarisse]

Ben Bacarisse said:



As far as I can recall, only two people whose opinions I respect
have ever advocated it, and then for rather specialised reasons
involving their specific code situation.

P J Plauger and B Stroustrup? I think it is worth stating, for the
record, that P J Plauger's advice is that most C programmers should
*not* cast malloc. He does not advocate it in general. As you say,
his situation is not a usual one.

In general, there is no good reason to add the cast,

Hmm... I suppose it depends on what counts as a good reason, but it
is sometimes needed in old C when the malloc call is an argument to
non-prototyped function (it depends on that type as actually
expected). The other situation that springs to mind is initialising a
union (in C90) whose first member is not a pointer compatible with
void *. To my mind these are good reasons but they fall outside the
scope of "in general" since they are rare situations.

Plauger points out that no rule should substitute for understanding
why one is doing something -- sound advice well worth repeating.

and several good reasons not to.

The fact that (in the usual situations) it is not needed and it
produces simpler code is enough for me. Are there any others?

 

[Flash Gordon]

An example please for those of us with limited imaginations.

For the side against casting malloc:

[markg@cpa-re-test ~]1002$ cat t.c
int main(void)
{
return malloc(9);
}
[markg@cpa-re-test ~]1003$ gcc t.c


This is on a server running a still commercially supported version of
Linux. The server in question is the one we use to do our official
software builds.

 

[Keith Thompson]

Hmm... I suppose it depends on what counts as a good reason, but it
is sometimes needed in old C when the malloc call is an argument to
non-prototyped function (it depends on that type as actually
expected). The other situation that springs to mind is initialising a
union (in C90) whose first member is not a pointer compatible with
void *. To my mind these are good reasons but they fall outside the
scope of "in general" since they are rare situations.

[...]

I don't understand the union case. For example, this:

union {
int *p;
double d;
} *obj = { malloc(sizeof obj->p) };

is valid as far as I can tell. (I presume the "(in C90)"
qualification is because in C90 you'd use a designated initializer
rather than depending on the first-member rule.)

 

[Kaz Kylheku]

P J Plauger and B Stroustrup? I think it is worth stating, for the
record, that P J Plauger's advice is that most C programmers should
*not* cast malloc. He does not advocate it in general. As you say,
his situation is not a usual one.


Hmm... I suppose it depends on what counts as a good reason, but it
is sometimes needed in old C when the malloc call is an argument to
non-prototyped function (it depends on that type as actually
expected).

A good reason to use a cast when converting from void * is that this is a
potentially unsafe conversion. It constitutes a hole in the type system, which
allows the programmer to take on the responsibility for correct typing,
such that there is no easy way to identify the places where this is happening
in the program: there is no diagnosis for those places, nor are they flagged
with an explicit syntax that can be searched for.

The ideal way to resolve the situation is to a) add the diagnostic to the
compiler, so that implicit unsafe conversions are flagged, and then b) to add
the explicit conversion operator to all of the places that are consequently
flagged, once they have been reviewed and regarded as safe. Then the diagnostic
will catch future situations where the conversion is introduced, but not
denoted by explicit syntax.

The second best thing, if you can't do a), is to just do b) anyway: add the
casts.

One day, the C standard may close the hole, like C++ did, making the cast
required. Programs that already have the cast are ``future proofed''
against this situation. The diagnostic may be available in existing some
compilers as an option; by using the cast you are writing code that will
not raise false positives when it's used in a project whose maintainers
decide to turn this on.

The fact that (in the usual situations) it is not needed and it
produces simpler code is enough for me. Are there any others?

Considering only what is strictly needed isn't always good engineering.

Correctly written assert expressions aren't needed; we add them anyway.

A function prototype declaration isn't needed. The number and types of
arguments can be deduced from the call to the function, so the prototype
is redundant. Just get the call right, and all is well.

Superfluous parentheses are not needed; just memorize the precedence table.

In writing software, we take lots of steps and measures that aren't strictly
needed.

The ``not needed'' argument is weak.

 

[Kaz Kylheku]

Ben Bacarisse said:


One out of two ain't bad. Plauger is the obvious one. I didn't know
that Stroustrup advocates adding the cast to malloc. I thought he
advocated not using malloc at all!

Obviously, Stroustrup designed a language based on C in which he banished
the void * type system hole, making the cast required. So it would
be surprising if he didn't think that using a cast when converting out
of void * is a good idea.