Page MenuHomeFreeBSD
Differential D17153

rtld-elf: fix WANRS=4 warnings other than -Wcast-align
ClosedPublic
Actions

Authored by arichardson on Sep 13 2018, 2:45 PM.
Tags
None
Referenced Files
F135970689: D17153.id49770.diff
Fri, Nov 14, 4:47 PM
F135927567: D17153.id48003.diff
Fri, Nov 14, 6:03 AM
Unknown Object (File)
Fri, Nov 14, 3:29 AM
Unknown Object (File)
Thu, Nov 13, 10:08 PM
Unknown Object (File)
Thu, Nov 13, 7:22 PM
Unknown Object (File)
Thu, Nov 13, 1:08 PM
Unknown Object (File)
Tue, Nov 11, 4:57 PM
Unknown Object (File)
Thu, Nov 6, 10:40 PM
View All Files
Subscribers
andrew
emaste
imp

Details

Reviewers
kib
jhb
brooks
Commits
rS339878: rtld-elf: compile with WANRS=4 warnings other than -Wcast-align
Summary

I didn't update the makefile since I have only tested compiled amd64 so far.

Test Plan

compiles fine

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

arichardson created this revision.Sep 13 2018, 2:45 PM
Harbormaster completed remote builds in B19567: Diff 48003.Sep 13 2018, 2:45 PM
kib accepted this revision.Sep 13 2018, 3:46 PM

I only listed style nits. Some of them are yours, some are not but could be fixed since you are changing the lines anyway.

libexec/rtld-elf/amd64/reloc.c
70 ↗(On Diff #48003)

Remove space between ') ('.

libexec/rtld-elf/rtld.c
802 ↗(On Diff #48003)

More spaces.

960 ↗(On Diff #48003)

char *. This is systematic.

1223 ↗(On Diff #48003)

Elf_Dyn *. Also remove space before &r_debug.

3208 ↗(On Diff #48003)

Fix indent to 4 spaces.

5129 ↗(On Diff #48003)

Space after type () in cast, there and below.

This revision is now accepted and ready to land.Sep 13 2018, 3:46 PM
brooks accepted this revision.Sep 13 2018, 4:16 PM

Module to style issues kib points out, this looks good to me.

libexec/rtld-elf/rtld.c
4473 ↗(On Diff #48003)

The use of tabs here is inconsistent with the rest of the file (at least that I see in phab).

arichardson updated this revision to Diff 49468.Oct 22 2018, 9:39 PM
arichardson marked 4 inline comments as done.
  • addressed style issues
  • ran make universe and fixed all warnings in */reloc.c
  • Bumped WARNS to 4
  • silenced -Wshadow for GCC 4.2 which warns about index()
This revision now requires review to proceed.Oct 22 2018, 9:39 PM
Herald added a subscriber: emaste. · View Herald TranscriptOct 22 2018, 9:39 PM
Harbormaster completed remote builds in B20339: Diff 49468.Oct 22 2018, 9:39 PM
arichardson marked 2 inline comments as done.Oct 22 2018, 9:45 PM
arichardson updated this revision to Diff 49471.Oct 22 2018, 10:08 PM

Fix error building aarch64

Harbormaster completed remote builds in B20341: Diff 49471.Oct 22 2018, 10:08 PM
kib accepted this revision.Oct 23 2018, 4:29 PM
kib added inline comments.
libexec/rtld-elf/aarch64/reloc.c
29 ↗(On Diff #49471)

Traditionally there is the blank line between license and first statement.

54 ↗(On Diff #49471)

This blank line ends the scope of multi-line comment above. I do not think that it is relevant for _exit ?

129 ↗(On Diff #49471)

Move this to top, where other function prototypes are located ?

libexec/rtld-elf/i386/reloc.c
70 ↗(On Diff #49471)

Still one more extra space, before dstobj->rel.

149 ↗(On Diff #49471)

And there.

243 ↗(On Diff #49471)

Space between Obj_Entry and '*' is needed.

libexec/rtld-elf/riscv/reloc.c
323 ↗(On Diff #49471)

space before '*'.

350 ↗(On Diff #49471)

and there.

libexec/rtld-elf/rtld.c
801 ↗(On Diff #49471)

No space after ).

This revision is now accepted and ready to land.Oct 23 2018, 4:29 PM
arichardson updated this revision to Diff 49523.Oct 23 2018, 6:19 PM
arichardson marked 9 inline comments as done.

More coding style fixes

This revision now requires review to proceed.Oct 23 2018, 6:19 PM
Harbormaster completed remote builds in B20367: Diff 49523.Oct 23 2018, 6:20 PM
arichardson added a comment.Oct 23 2018, 6:21 PM

Thanks for reviewing this annoyingly big diff and finding all the style issues. Too bad we can't just use git-clang-format :(

libexec/rtld-elf/aarch64/reloc.c
54 ↗(On Diff #49471)

This was accidentally commited. Will fix

129 ↗(On Diff #49471)

I tried this but then it complains about struct tls_data not being visible outside this function declaration.
Obviously easily fixed by forward declaring `struct tls_data, but I moved it below the struct definition instead since I wasn't sure in which order to put the forward declaration of the struct.

Not sure which solution is preferable but I'm happy to move it to the top again

kib added inline comments.Oct 23 2018, 6:36 PM
libexec/rtld-elf/aarch64/reloc.c
129 ↗(On Diff #49471)

Ok, leave it as is, just commit the review.

brooks accepted this revision.Oct 29 2018, 8:49 PM
This revision is now accepted and ready to land.Oct 29 2018, 8:49 PM
Closed by commit rS339878: rtld-elf: compile with WANRS=4 warnings other than -Wcast-align (authored by arichardson). · Explain WhyOct 29 2018, 9:08 PM
This revision was automatically updated to reflect the committed changes.
Herald added subscribers: andrew, imp. · View Herald TranscriptOct 29 2018, 9:08 PM

Revision Contents
Changeset List

PathSize
head/
libexec/
rtld-elf/
9 lines
aarch64/
22 lines
amd64/
27 lines
arm/
26 lines
i386/
23 lines
2 lines
8 lines
7 lines
16 lines
mips/
22 lines
3 lines
10 lines
powerpc/
35 lines
1 line
powerpc64/
33 lines
1 line
riscv/
32 lines
6 lines
137 lines
sparc64/
37 lines

Diff 49770

View Options

head/libexec/rtld-elf/Makefile

Loading...
View Options

head/libexec/rtld-elf/aarch64/reloc.c

Loading...
View Options

head/libexec/rtld-elf/amd64/reloc.c

Loading...
View Options

head/libexec/rtld-elf/arm/reloc.c

Loading...
View Options

head/libexec/rtld-elf/i386/reloc.c

Loading...
View Options

head/libexec/rtld-elf/i386/rtld_machdep.h

Loading...
View Options

head/libexec/rtld-elf/libmap.c

Loading...
View Options

head/libexec/rtld-elf/malloc.c

Loading...
View Options

head/libexec/rtld-elf/map_object.c

Loading...
View Options

head/libexec/rtld-elf/mips/reloc.c

Loading...
View Options

head/libexec/rtld-elf/mips/rtld_machdep.h

Loading...
View Options

head/libexec/rtld-elf/paths.h

Loading...
View Options

head/libexec/rtld-elf/powerpc/reloc.c

Loading...
View Options

head/libexec/rtld-elf/powerpc/rtld_machdep.h

Loading...
View Options

head/libexec/rtld-elf/powerpc64/reloc.c

Loading...
View Options

head/libexec/rtld-elf/powerpc64/rtld_machdep.h

Loading...
View Options

head/libexec/rtld-elf/riscv/reloc.c

Loading...
View Options

head/libexec/rtld-elf/rtld.h

Loading...
View Options

head/libexec/rtld-elf/rtld.c

Loading...
View Options

head/libexec/rtld-elf/sparc64/reloc.c

Loading...