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

[Jan Beulich <jbeulich@suse.com>]

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.

Jan