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
F106132335: D19715.diff
Wed, Dec 25, 10:42 PM
Unknown Object (File)
Fri, Dec 13, 5:23 AM
Unknown Object (File)
Wed, Dec 11, 4:30 PM
Unknown Object (File)
Oct 2 2024, 12:49 PM
Unknown Object (File)
Oct 2 2024, 12:37 AM
Unknown Object (File)
Sep 30 2024, 5:29 PM
Unknown Object (File)
Sep 23 2024, 3:35 AM
Unknown Object (File)
Sep 22 2024, 8:16 PM
View All 50 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 23307
Build 22338: 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
904

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

905

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

933

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
933

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
933

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/
124 lines

Diff 55458

View Options

sys/kern/imgact_elf.c

Loading...