Re: [PATCH] ld: Sort section contributions in PDB files

[Alan Modra <amodra@gmail.com>]

On Tue, Feb 21, 2023 at 09:26:33AM +0100, Jan Beulich wrote:
> On 20.02.2023 15:13, Mark Harmstone wrote:
> > +/* Used as parameter to qsort, to sort section contributions by section and
> > +   offset.  */
> > +static int
> > +section_contribs_compare (const void *p1, const void *p2)
> > +{
> > +  const struct in_sc *sc1 = (const struct in_sc *) p1;
> > +  const struct in_sc *sc2 = (const struct in_sc *) p2;
> 
> In ANSI C there's no need for these casts; it may be that they were
> needed in pre-ANSI dialects like K&R. Personally I view _any_ cast
> as latently dangerous, and hence I'd prefer if casts were used only
> if there's no other option.

I agree that it's fine to write this without the casts, and I've even
used the cast to void* you mention later in your email to shorten
lines myself.  I agree that casts are inherently dangerous too, and
that it's a good idea to not use them unless necessary.  Also, it's
really, really annoying to need casts because something like
  os = &lang_os_list.head->output_section_statement;
gets a ubsan warning when lang_os_list.head is NULL, forcing you to
use a cast or accessor that loses the type checking or to write
horrendous code.  There have been bugs in ld list handling due to
casts.

However, I'm inclined to say that a cast in a qsort comparison
function, or to cast the return of malloc or similar is mostly a
matter of style.  If a contributor wants to write it that way, I'm
fine with approving new code with these casts.  After all, there is
plenty of such code in binutils already.

-- 
Alan Modra
Australia Development Lab, IBM