Split kern_break from sys_break and use it in linuxulator
ClosedPublic
Actions

Authored by emaste on Jun 26 2018, 6:17 PM.

Details

Reviewers

kib
brooks

Commits

rS335702: Split kern_break from sys_break and use it in linuxulator

Summary

Previously the linuxulator's linux_brk invoked the FreeBSD sys_break syscall implementation directly. Instead, move the bulk of the existing implementation to kern_break, and use that from both sys_break and linux_brk.

This also addresses a minor bug in linux_brk in that we now return the actual (rounded up) break address, rather than the requested value.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

emaste created this revision.Jun 26 2018, 6:17 PM

Looks good to me. I wonder if we also want to restore ibcs2's int return value?

brooks accepted this revision.Jun 26 2018, 10:46 PM

This revision is now accepted and ready to land.Jun 26 2018, 10:46 PM

kib added inline comments.Jun 27 2018, 7:16 AM

sys/compat/linux/linux_misc.c
243 ↗	(On Diff #44472)	Remove excess ().
244 ↗	(On Diff #44472)	BTW, type of td_retval is register_t, not long.
sys/vm/vm_unix.c
76 ↗	(On Diff #44472)	Excess (). And the cast too.
87 ↗	(On Diff #44472)	Mismatched types: prototype uses uintptr_t *addr.

Review feedback from @kib

This revision now requires review to proceed.Jun 27 2018, 12:42 PM

emaste added inline comments.Jun 27 2018, 12:43 PM

sys/vm/vm_unix.c
76 ↗	(On Diff #44472)	What about the cast?

I wonder if we also want to restore ibcs2's int return value?

Would that be implementing a ibcs2-specific wrapper? I suspect it's not worth it.

emaste added inline comments.Jun 27 2018, 2:10 PM

sys/vm/vm_unix.c
76 ↗	(On Diff #44519)	typo, corrected locally to `uap->nsize`

This revision was not accepted when it landed; it landed in state Needs Review.Jun 27 2018, 2:45 PM

Closed by commit rS335702: Split kern_break from sys_break and use it in linuxulator (authored by emaste). · Explain Why

This revision was automatically updated to reflect the committed changes.

Herald added a subscriber: imp. · View Herald TranscriptJun 27 2018, 2:45 PM

In D16019#339574, @emaste wrote:

I wonder if we also want to restore ibcs2's int return value?

Would that be implementing a ibcs2-specific wrapper? I suspect it's not worth it.

I agree it probably isn't worth it.