Re: What's in git.git

[Linus Torvalds <torvalds@osdl.org>]

On Sun, 25 Jun 2006, Junio C Hamano wrote:
> 
>    Timo Hirvonen:
>       Clean up diff.c

THIS IS CRAP!

Dammit, anybody who claims that casting a constant string to "(char *)" is 
a _cleanup_ is doing something seriously wrong.

That's crap, crap, crap, CRAP!

If the "cleanup" was about hiding compiler warnings, then dammit, those 
warnings should be fixed by fixing the code, not by casting the warning 
away but leaving the broken code.

If the ptr really is never accessed, and doesn't matter, then don't use a 
constant empty string, use NULL.

And if it _is_ accessed, then casting a constant string to "char *" is 
_wrong_. 

The whole and only point about the "const" warnings is not to hide them, 
but to fix the code. If you're not going to fix the code, then you 
shouldn't ask the compiler to warn about it, it's that simple. Adding 
bogus casts is not the answer.

I really hate how many _bogus_ casts we're growing. Casts are one of the 
most important features of C (it's what allows you to break the type 
system if you need to, and turns C into the truly extensible language it 
is), but they should be used with reverence and care, not to shut up a 
compiler.

I'm _especially_ disgusted by how this was claimed to be a "cleanup". 
Adding a cast is _never_ a cleanup.

Dammit, don't do crap like this!

THIS is a cleanup:

-               char *prefix = "";
+               const char *prefix = "";

but THESE are total and utter CRAP:

-               mf->ptr = ""; /* does not matter */
+               mf->ptr = (char *)""; /* does not matter */
-                               s->data = "";
+                               s->data = (char *)"";

and we're better off with the warning than with the new code.

I suspect that both could have been made to use NULL instead to indicate 
that no pointer exists.

			Linus