Page MenuHomeFreeBSD
Differential D2193

linprocfs_domtab() rewrite
ClosedPublic
Actions

Authored by trasz on Apr 1 2015, 8:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 30, 8:30 AM
Unknown Object (File)
Tue, Dec 24, 8:56 AM
Unknown Object (File)
Sat, Dec 21, 3:52 PM
Unknown Object (File)
Tue, Dec 17, 6:31 AM
Unknown Object (File)
Dec 11 2024, 2:35 AM
Unknown Object (File)
Nov 30 2024, 8:48 AM
Unknown Object (File)
Nov 27 2024, 7:17 AM
Unknown Object (File)
Nov 17 2024, 9:42 AM
View All Files
Subscribers
emaste
imp

Details

Reviewers
kib
Group Reviewers
Linux Emulation
Commits
rS281551: Rewrite linprocfs_domtab() as a wrapper around kern_getfsstat(). This
Summary

Rewrite linprocfs_domtab() as a wrapper around kern_getfsstat(). This
adds missing jail and MAC checks.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

trasz updated this revision to Diff 4562.Apr 1 2015, 8:07 AM
trasz retitled this revision from to linprocfs_domtab() rewrite.
trasz updated this object.
trasz edited the test plan for this revision. (Show Details)
Herald added a subscriber: imp. · View Herald TranscriptApr 1 2015, 8:07 AM
trasz added a reviewer: Linux Emulation.Apr 1 2015, 8:09 AM
trasz added a reviewer: kib.Apr 1 2015, 8:14 AM
kib edited edge metadata.Apr 1 2015, 9:34 AM

This is most likely a good change.

sys/compat/linprocfs/linprocfs.c
335

Do not initialize in definition.

It is probably safer to initialize the buf to NULL (not required, but it made me wonder for a moment).

342

I see that this is not changed, but why is error from namei() ignored ?

kib added a comment.Apr 1 2015, 9:42 AM

It seems reasonable to change the kern_getfsstat() to return the count explicitely, instead of relying on td_retval. I think it was only used by compat32 so far, but new use is ugly without the proper handling of count.

trasz updated this revision to Diff 4568.Apr 1 2015, 10:23 AM
trasz edited edge metadata.

Tidy up.

trasz added a comment.Apr 1 2015, 10:24 AM

I think the error is ignored to account for possibility that the path doesn't exist. I'd prefer not to touch it for now, this code is a little scary.

As for returning the count from kern_getfsstat() - good point, but I'd rather leave this for later.

emaste added a subscriber: emaste.Apr 1 2015, 2:31 PM
trasz added a comment.Apr 2 2015, 10:20 AM

So, does it look ok now? If so, can you mark it Accepted? Thanks!

kib added a comment.Apr 2 2015, 8:20 PM

I do not like td_retval thing, it is too hackish. IMO this is exactly the right moment to fix it.

I agree that handling namei error (or commenting why error could be ignored) is for the other moment.

trasz added a comment.Apr 3 2015, 5:38 PM

Hm, okay. How to return the count, then? Reuse the 'count' argument, or add a new one? Is there a convention that says which argument should it be? The last?

kib added a comment.Apr 3 2015, 6:15 PM
In D2193#13, @trasz wrote:

Hm, okay. How to return the count, then? Reuse the 'count' argument, or add a new one? Is there a convention that says which argument should it be? The last?

Up to you and a common sense. I would add the last argument like int *count (exact type might need some correction).

trasz updated this revision to Diff 4812.Apr 14 2015, 7:54 AM

Make kern_getfsstat() return count explicitly.

kib added a comment.Apr 14 2015, 7:58 AM

Yes, this is the intended change, but it seems that you missed compat/freebsd32.

trasz updated this revision to Diff 4813.Apr 14 2015, 10:58 AM

Don't miss compat32.

kib added inline comments.Apr 14 2015, 11:53 AM
sys/compat/freebsd32/freebsd32_misc.c
258

Move the assignment after we ensured that all copyout()s succeeded.

sys/kern/vfs_syscalls.c
442

Do not assign to td_retval if error != 0.

trasz updated this revision to Diff 4815.Apr 14 2015, 12:06 PM

Fix retval handling.

kib added inline comments.Apr 14 2015, 12:08 PM
sys/kern/vfs_syscalls.c
634

There too ?

trasz updated this revision to Diff 4816.Apr 14 2015, 12:15 PM

Add one more missing error check.

kib accepted this revision.Apr 14 2015, 12:28 PM
kib edited edge metadata.
This revision is now accepted and ready to land.Apr 14 2015, 12:28 PM
trasz closed this revision.Apr 15 2015, 9:13 AM
trasz updated this revision to Diff 4833.

Closed by commit rS281551 (authored by @trasz).

Revision Contents
Changeset List

PathSize
sys/
compat/
freebsd32/
4 lines
linprocfs/
37 lines
kern/
19 lines
sys/
2 lines

Diff 4813

View Options

sys/compat/freebsd32/freebsd32_misc.c

Loading...
View Options

sys/compat/linprocfs/linprocfs.c

Loading...
View Options

sys/kern/vfs_syscalls.c

Loading...
View Options

sys/sys/syscallsubr.h

Loading...