Page MenuHomeFreeBSD
Differential D19715

Factor out retrieving the interpreter path from the main ELF loader routine.
ClosedPublic
Actions

Authored by trasz on Mar 26 2019, 3:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 20 2023, 2:22 AM
Unknown Object (File)
Nov 22 2023, 10:28 PM
Unknown Object (File)
Nov 22 2023, 10:27 PM
Unknown Object (File)
Nov 22 2023, 10:02 PM
Unknown Object (File)
Nov 22 2023, 8:58 PM
Unknown Object (File)
Nov 19 2023, 10:15 AM
Unknown Object (File)
Nov 14 2023, 5:11 AM
Unknown Object (File)
Nov 13 2023, 2:13 PM
View All 25 Files
Subscribers
imp

Details

Reviewers
kib
Commits
rS345661: Factor out retrieving the interpreter path from the main ELF
Summary

Factor out retrieving the interpreter path from the main ELF
loader routine.

One small difference is that it no longer errors out if there
are multiple PT_INTERP sections; it just uses the first one.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 23328
Build 22358: arc lint + arc unit

Event Timeline

trasz created this revision.Mar 26 2019, 3:42 PM
Herald added a subscriber: imp. · View Herald TranscriptMar 26 2019, 3:42 PM
Harbormaster completed remote builds in B23307: Diff 55458.Mar 26 2019, 3:42 PM
kib added a comment.Mar 26 2019, 4:08 PM

If you only move some parts of the case PT_INTERP into the helper, you can 1. avoid one more pass over phdrs 2. easily keep the logic to error on more than one interpreter.

Also I do not completely like the removal of the interp_name_len argument from the get_brandinfo args.

sys/kern/imgact_elf.c
285

If you insist on doing this arg removal, at least write this code as

interp_name_len = interp != NULL ? strlen(interp) : 0;
trasz updated this revision to Diff 55461.Mar 26 2019, 4:34 PM

Kib's suggestions.

Updating D19715: Factor out retrieving the interpreter path from the main ELF

loader routine.

Harbormaster completed remote builds in B23309: Diff 55461.Mar 26 2019, 4:34 PM
kib added inline comments.Mar 26 2019, 5:05 PM
sys/kern/imgact_elf.c
901

Add assertions: phdr type is PT_INTERP, vp is exclusively locked.

902

curthread is used only once, it seems. I do not see a need for td.

930

If you restore the 'else' clause, then *interpp = interp; return (0); can be left once in the end of the function.

trasz updated this revision to Diff 55497.Mar 27 2019, 2:45 PM

Improve.

Updating D19715: Factor out retrieving the interpreter path from the main ELF

loader routine.

Harbormaster completed remote builds in B23328: Diff 55497.Mar 27 2019, 2:46 PM
trasz marked 2 inline comments as done.Mar 27 2019, 2:46 PM
trasz added inline comments.
sys/kern/imgact_elf.c
930

I find the current version easier to read, to be honest.

kib accepted this revision.Mar 27 2019, 4:41 PM
kib added inline comments.
sys/kern/imgact_elf.c
930

For me (the reader) it is reverse. I need to understand that all successful exit paths satisfy the function interface, and single success-exit with non-duplicated code makes it easier to see.

I would not stop you from committing this due to the rearrangement of three lines, but I prefer the described version.

This revision is now accepted and ready to land.Mar 27 2019, 4:41 PM
Closed by commit rS345661: Factor out retrieving the interpreter path from the main ELF (authored by trasz). · Explain WhyMar 28 2019, 9:43 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
kern/
115 lines

Diff 55497

View Options

sys/kern/imgact_elf.c

Loading...