I didn't update the makefile since I have only tested compiled amd64 so far.
Details
compiles fine
Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 19567 Build 19152: arc lint + arc unit
Event Timeline
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 | Remove space between ') ('. | |
libexec/rtld-elf/rtld.c | ||
802 | More spaces. | |
960 | char *. This is systematic. | |
1223 | Elf_Dyn *. Also remove space before &r_debug. | |
3208 | Fix indent to 4 spaces. | |
5129 | Space after type () in cast, there and below. |
Module to style issues kib points out, this looks good to me.
libexec/rtld-elf/rtld.c | ||
---|---|---|
4473 | The use of tabs here is inconsistent with the rest of the file (at least that I see in phab). |
- 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 | ||
800 | No space after ). |
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. |