Re: mdocml: To remove the const qualifier from a pointer to an object

[Ingo Schwarze <schwarze@usta.de>]

Hi Joerg,

we seem to disagree here.

Joerg Sonnenberger wrote on Fri, Jul 15, 2016 at 11:31:03PM +0200:
> On Fri, Jul 15, 2016 at 02:33:31PM -0500, schwarze@mdocml.bsd.lv wrote:

>> To remove the const qualifier from a pointer to an object - either 
>> because we know it is actually mutable or because we are passing
>> it to a function that doesn't accept a const object but won't
>> actually attempt to modify it - simply casting from (const type *)
>> to (type *) is legal C and clearly expresses the intent.  
>> So get rid of the obfuscating UNCONST macro.
>> Basic idea discussed with guenther@.

> This is completely wrong and a clear regression.

There is no regression.  The code works fine after the change.

> The macro exists

The macro does not exist.  Neither the C standard nor POSIX specify
it or even indicate that something like that might be needed or
even useful.  It was a purely local quirk in one single place of
the mandoc codebase.  Besides, i checked with Philip Guenther, who
is both an expert in C and in POSIX, that no contortions are needed
here.

> for two reasons:
> (1) It makes auditing easier by giving something simple to look for.

It makes auditing harder due to obfuscation.  What a plain cast
does is obvious on first sight.  A macro needs to be looked up when
auditing the code to understand what it does, and due to the
unnecessary quadruple cast const char **" -> const void * ->
intptr_t -> void * -> char * const * instead of one single cast
it makes auditing even harder.

I see no point in specifically auditing for casts removing const.
Sure, auditing for type conversion issues is useful, but many type
conversions, particularly implicit ones, are much more dangerous
than const removals.  Auditing for type conversion errors is very
hard (think of signedness and width issues).  Introducing an ugly
macro to make one of the lesser problems stand out prominently
doesn't help at all.

> (2) The macro actually stopped -Wcast warnings.

You mean, -Wcast-qual?  That's not enabled by default for a reason:
It's not useful in production builds.  (It may occasionally be
helpful when doing a one-time audit of a dubious code base to
identify candidates for erroneous casts, though.)

> The replacement does not.

Just don't enable -Wcast-qual for compiling mandoc.  I consider
that option harmful because it drives developers toward obfuscation.

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv