[PATCH 0/7] undoing some const patches
peter.hutterer at who-t.net
Wed Jan 29 15:51:13 PST 2014
A bunch of fixes to undo some of the damage from the const warning cleanup.
Keith, I have to say fixing this made me sad. Those warning fixes I looked
at are misguided at best and wrong at worst. All of the commit messages
merely say "fixes warnings" with no indication of what the actual warnings
were. And as far as I can tell, you didn't look in much detail either.
For example, in fecc7eb1cf66db64728ee2d68cd9443df7e70879 you made the
DeviceIntRec's name a const char*. AFAICT the only place this issued a
warning was in the test code where a temporary device got assigned a fixed
string. Instead of fixing a couple of 20-line test cases you changed one of
the core structs of the server.
The patch that makes xstrtokenize return a const char** when the comment
right above says "Always returns an allocated array unless an error occurs."
... I mean... seriously? what was the rationale for that?
I stopped after that because the next thing I wanted to revert was another
const char *name change. Fixing that caused another warning, which was
caused by patches adding more const chars all over the place and it was a
rabbit hole after that.
>From the couple of patches I've looked at in detail now I have to say most
are wrong. Yes, they fix the warning, but from a general coding perspective
it's wrong. Jasper pointed this out, Thierry pointed this out, Ajax
mentioned this. You argument was "the decision to free is generally based on
external knowledge of whether the storage was allocated or not." Judging
from the patches I looked at, that external knowledge is also very doubtful.
If you really don't want to see warnings, disable your warning flags because
quite frankly I see little difference between that and those patches. One
thing that's true for me now is that "const char*" in the server doesn't
mean anything anymore and is now merely a "shut up compiler" type. And that
makes the code harder to read, harder to understand, and harder to write.
Maybe all this is different in the output code, but somewhat I doubt it.
More information about the xorg-devel