I didn't update the makefile since I have only tested compiled amd64 so far.
Details
Details
compiles fine
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
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. |
Comment Actions
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). |
Comment Actions
- 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()
| 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 ). |
Comment Actions
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. Not sure which solution is preferable but I'm happy to move it to the top again |
| libexec/rtld-elf/aarch64/reloc.c | ||
|---|---|---|
| 129 ↗ | (On Diff #49471) | Ok, leave it as is, just commit the review. |