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

[Alan Modra <amodra@gmail.com>]

On Wed, Feb 22, 2023 at 08:08:43AM +0100, Jan Beulich wrote:
> On 22.02.2023 06:48, Alan Modra wrote:
> > 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.
> 
> Now that's an interesting perspective. Depending on feedback on the
> topic I was meaning to possibly conclude that such unnecessary casts
> would be ripe for removal. Perhaps not by hunting them down and
> replacing them in an isolated effort, but as code is being touched
> anyway.

I'm fine with doing that too, particularly as you touch code.

What I was trying to say, is that I think it's important for
maintainers to allow contributors some freedom in style, to tolerate
things that don't matter that much.

-- 
Alan Modra
Australia Development Lab, IBM