readelf(1): Speed up readelf -wo

[borako.ozarslan_gmail.com]

Summary

Speed up readelf -wo by using an array instead of STAILQ and instead of adding new elements ordered, sorting them after the list is complete.

PR: 212539
Submitted by: Bora Ozarslan [e-mail]

[markj]

Sorry it took a while to get to this. Most of my comments are style nits.

readelf/readelf.c

5923

"comparator"?

5926

style(9) disallows mixed declaration and initialization.

5934

Per style(9), continuation lines should be indented by four spaces.

6000–6001

style: there should be a space after control-flow keywords.

6001

Memory allocation failures should be handled to be consistent within this program. Ditto below.

6008

This can be written as *la_list_len++;.

6377–6398

BSD style(9) discourages C++ comments in C code.

Comments should be full sentences.

6383

style(9) discourages nested declarations; these should be declared at the beginning of the function.

6392

Are we leaking la_list here?

It also looks as though this check should logically come before we attempt to sort and remove duplicates.

[markj]

I think you uploaded the diff between the first and second revision rather than the revision as a whole - could you upload the full diff?

[borako.ozarslan_gmail.com]

Merged both of the diffs

[borako.ozarslan_gmail.com]

Get rid of extra semicolon

[markj]

contrib/elftoolchain/readelf/readelf.c

6051 ↗

(On Diff #54088)

Style: missing newline after declarations.

6120 ↗

(On Diff #54088)

The indentation should be by four spaces.

6127 ↗

(On Diff #54088)

I'd suggest using a local variable for (*la_list)[*la_list_len].

6143 ↗

(On Diff #54088)

Style: indentation.

6151 ↗

(On Diff #54088)

Indentation.

6440 ↗

(On Diff #54088)

Extra calloc() call.

6464 ↗

(On Diff #54088)

Indentation.

6513 ↗

(On Diff #54088)

Style: missing space after for and if below.

For consistency with the rest of the code, postfix increment should be used in loop headers in places where it doesn't matter.

6518 ↗

(On Diff #54088)

You can just write duplicates++.

6522 ↗

(On Diff #54088)

Don't you want to subtract duplicates from la_list_len here?

[borako.ozarslan_gmail.com]

Fix style issues

[markj]

Looks ok to me, modulo a couple more style nits.

contrib/elftoolchain/readelf/readelf.c

6073 ↗

(On Diff #54225)

This declaration should be at the top: wider types come first.

6125 ↗

(On Diff #54225)

errx() doesn't return, so the return statement is redundant.

[emaste]

contrib/elftoolchain/readelf/readelf.c

6056 ↗

(On Diff #54232)

This feels too clever to me; would prefer a standard

if (left->la_off > right->la_off)
        return (1);
else if (left->la_off < right->la_off)
        return (-1);
else
        return (0);

markj@, your thoughts?
(I can just apply that change locally upon commit.)

[markj]

contrib/elftoolchain/readelf/readelf.c

6056 ↗

(On Diff #54232)

I prefer your version too.

[emaste]

rS346323