I didn't update the makefile since I have only tested compiled amd64 so far.
Details
Details
compiles fine
Diff Detail
Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 20367 Build 19808: arc lint + arc unit
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 | Remove space between ') ('. | |
| libexec/rtld-elf/rtld.c | ||
| 803 | More spaces. | |
| 961 | char *. This is systematic. | |
| 1224 | Elf_Dyn *. Also remove space before &r_debug. | |
| 3222–3223 | Fix indent to 4 spaces. | |
| 5144 | 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 | ||
|---|---|---|
| 4487–4488 | 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 | Traditionally there is the blank line between license and first statement. | |
| 54 | This blank line ends the scope of multi-line comment above. I do not think that it is relevant for _exit ? | |
| 131 | Move this to top, where other function prototypes are located ? | |
| libexec/rtld-elf/i386/reloc.c | ||
| 70 | Still one more extra space, before dstobj->rel. | |
| 149 | And there. | |
| 243 | Space between Obj_Entry and '*' is needed. | |
| libexec/rtld-elf/riscv/reloc.c | ||
| 323 | space before '*'. | |
| 350 | and there. | |
| libexec/rtld-elf/rtld.c | ||
| 801 | 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 | This was accidentally commited. Will fix | |
| 131 | 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 | ||
|---|---|---|
| 131 | Ok, leave it as is, just commit the review. | |