Page MenuHomeFreeBSD

Sandbox head(1) with fileargs.
ClosedPublic

Authored by oshogbo on Feb 17 2018, 12:28 PM.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

oshogbo created this revision.Feb 17 2018, 12:28 PM
oshogbo added a reviewer: bapt.
emaste added inline comments.Feb 17 2018, 1:01 PM
usr.bin/head/head.c
47 ↗(On Diff #39414)

If either <sys/types.h> or <sys/param.h> is needed, include it before other include files.

from style(9)

Still sys/cdefs.h first.

136 ↗(On Diff #39414)

Should we call fileargs_free?

oshogbo updated this revision to Diff 39419.Feb 17 2018, 1:29 PM

Add suggestion from @emaste .

oshogbo marked 2 inline comments as done.Feb 17 2018, 1:30 PM
oshogbo updated this revision to Diff 39587.Feb 21 2018, 9:58 PM

We handle now empty argc and argv.

emaste added inline comments.Feb 22 2018, 12:23 AM
usr.bin/head/head.c
81–82 ↗(On Diff #39587)

These additions maintain non-style(9) of variable declarations.

It's a pedantic comment, but as this change will likely be used as a reference for future fileargs additions we may want to address it; if we do, we should probably style(9) head.c first in a separate commit.

oshogbo added inline comments.Feb 26 2018, 7:07 PM
usr.bin/head/head.c
81–82 ↗(On Diff #39587)

I created another review for making it more style(9) https://reviews.freebsd.org/D14498

oshogbo updated this revision to Diff 43156.May 30 2018, 6:33 PM

Fix build without casper

This revision was not accepted when it landed; it landed in state Needs Review.Nov 12 2018, 5:48 PM
This revision was automatically updated to reflect the committed changes.
mmel added a subscriber: mmel.Dec 4 2018, 2:04 PM

How this can works on kernels build without 'options CAPABILITIES'? Note that CAPABILITIES option is
not included in kernel option for small embedded boards (arm, mips...)?
We should change CAPSICUM to mandatory or implement fallback here (as we already do in rest of tree)

Also, how this can be complied with WITHOUT_CAPSICUM=YES in make.conf? (imho, nanobsd uses it)

(same for wc and brandelf)

What you afraid of?
If you build system without capsicum/casper everything works caph_enter_casper don't fail in that case, the filearsgs_open is changed to the standard open(2) call>
Please check ot this header file: https://svnweb.freebsd.org/base/head/lib/libcasper/services/cap_fileargs/cap_fileargs.h?view=markup

mmel added a comment.Dec 4 2018, 2:41 PM

Ahh, right, I overlooked this, sorry. So I taking back WITHOUT_CAPSICUM case.

But first part on my previous message is still applicable, if you run 'head' on
kernel compiled without 'option CAPSICUM' then it fail with 'function not implemented'.

cem added a comment.Dec 4 2018, 5:02 PM
In D14409#392398, @mmel wrote:

if you run 'head' on
kernel compiled without 'option CAPSICUM' then it fail with 'function not implemented'.

Why do you think that? Maybe you don't have r341347?

mmel added a comment.Dec 5 2018, 5:05 AM

bingo! I'm on r341345. And yes, r341347 fixed it.
Thanks and sorry for noise.