On Fri, Feb 17, 2017 at 06:21:32PM +0100, Kamil Dudka wrote:
> On Friday, February 17, 2017 17:14:37 Andreas Mohr wrote:
> > 0. The code used an old-style BREAK-CAST *)
> > in order to work around (not: treat!) a transition issue caused by mismatch
> > of API signatures (ROOT CAUSE)
>
> I do not understand what you mean by BREAK-CAST. Assuming I am not the only
> one who does not understand it, putting it to the advisory would hardly help.

My point was that *every* cast is a breaking operation,
thus it should be precisely (and strongly) called out for the thing that it is: BREAK-CAST.
Breaking in that it completely disrupts type safety (as diagnosed and
thus maintained by a compiler),
which leads to issues in code,
either now already, or (as can be prominently seen in this textbook example)
at any time in any (bleak) future ;)

Any cast in code transitions introduces persisting weaknesses -
these weaknesses aren't relevant during development only
(where the developer will usually check that the cast as used "is ok"),
but also (and much more importantly!) during subsequent development where
conditions may suddenly change.

Once a cast is used, any compiler diagnosis is gone.
There are some possibly helpful options remaining, though:
- gcc cast warnings: -Wold-style-cast -Wcast-qual -Wcast-align , possibly others
- external static analysis tools (e.g. cppcheck, PVS)

One way to avoid a cast is to use unions
(however IIRC with unions there might be "aliasing" issues, see gcc).

> I also prefer to split commits that just move source code around from commits
> that are intended to change the actual behavior. Writing it to the advisory
> would make it overly chatty though. I think it is obvious that each single
> commit can be made better (for review, maintenance and everything) if you have
> enough manpower.

I didn't really think of amending the advisory.
However, if this was deemed useful, an official ("better" - ahem...) text
of my parts could be devised.

> > *) this is a bit of a stretch, since
> > with C language, this cast isn't old-style: there is that kind of cast only
> > (only C++ has "some" cast protection against the worst kinds of interface
> > transition abuses). In C, ways to still try to avoid destructive cast
> > issues might be: - (for sufficiently frequently invoked code parts) to
> > create some helper functions which do the casting internally, thus in a
> > controlled, maximally central manner - avoid API transition mismatch issues
>
> Which version of C++ supports "safe" cast of (char *) to (unsigned char *)?

Ouch. touché. :)

One might be able to do some "sufficiently safe/controlled" implementation
via a two-stage operation
via (de)reference stuff and simple static_cast:ing,
but frankly I am not sure without testing.

Andreas Mohr

-- 
GNU/Linux. It's not the software that's free, it's you.
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html