Clean up linux_getcwd().
ClosedPublic
Actions

Authored by trasz on Apr 18 2015, 7:27 PM.

Details

Reviewers

kib
dchagin

Group Reviewers

Linux Emulation

Commits

rS281882: Simplify linux_getcwd(), removing code that was longer used.

Summary

Simplify linux_getcwd(), removing code that seems to be no longer used.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

trasz updated this revision to Diff 4905.Apr 18 2015, 7:27 PM

trasz retitled this revision from to Clean up linux_getcwd()..

trasz updated this object.

trasz edited the test plan for this revision. (Show Details)

Herald added a subscriber: imp. · View Herald TranscriptApr 18 2015, 7:27 PM

trasz added reviewers: Linux Emulation, kib.Apr 18 2015, 7:28 PM

kib added inline comments.Apr 19 2015, 9:05 AM

sys/compat/linux/linux_getcwd.c
75 ↗	(On Diff #4905)	return (ERANGE);
84 ↗	(On Diff #4905)	How is it possible that the test is not satisfied ?

vn_fullpath may fail if intermediate entries in the path not in the cache.

sys/compat/linux/linux_getcwd.c
73 ↗	(On Diff #4905)	kern___getcwd() limits buflen to MAXPATHLEN

In D2326#5, @dchagin wrote:

vn_fullpath may fail if intermediate entries in the path not in the cache.

vn_fullpath() does what the removed code did, but probably with less bugs left. Only the last entry cache is critical to the operation, for the directories, the linear scan is performed if cache failed.

Update to handle LINUX_PATH_MAX limit correctly.

Split the non-linux part of the patch, for easy merging.

sys/compat/linux/linux_getcwd.c
77 ↗	(On Diff #4923)	This whole block is not needed, kern__getcwd() does the same checks.
82 ↗	(On Diff #4923)	if (error == 0) { ... Then you can avoid goto.
86 ↗	(On Diff #4923)	Do not assign to td_retval() if later copyout() fails.

trasz added inline comments.Apr 20 2015, 2:19 PM

sys/compat/linux/linux_getcwd.c
77 ↗	(On Diff #4923)	Yes, but we need to cap the "len" before we call malloc.

kib added inline comments.Apr 20 2015, 3:06 PM

sys/compat/linux/linux_getcwd.c
77 ↗	(On Diff #4923)	Always malloc LINUX_PATH_MAX bytes ?

dchagin added inline comments.Apr 20 2015, 3:53 PM

sys/compat/linux/linux_getcwd.c
47 ↗	(On Diff #4923)	this line should be below the line 55
77 ↗	(On Diff #4923)	also Linux returns ERANGE against our EINVAL, so leave it as is
85 ↗	(On Diff #4923)	kern__getcwd() can doit for you the same with UIO_USERSPACE

In D2326#7, @kostikbel wrote:

In D2326#5, @dchagin wrote:

vn_fullpath may fail if intermediate entries in the path not in the cache.

vn_fullpath() does what the removed code did, but probably with less bugs left. Only the last entry cache is critical to the operation, for the directories, the linear scan is performed if cache failed.

Thank you! Maybe it's time to correct vn_fullpath(9)

trasz added inline comments.Apr 20 2015, 5:06 PM

sys/compat/linux/linux_getcwd.c
85 ↗	(On Diff #4923)	If we use UIO_USERSPACE, how do we determine the path length then? We can't call strlen() for user buffer.

In D2326#15, @trasz wrote:

If we use UIO_USERSPACE, how do we determine the path length then? We can't call strlen() for user buffer.

yes, we can return it from kern__getcwd(), possibly via buflen?

Another try. The kern/ parts were committed earlier. Yeah,
we could avoid strlen and a copy, but kern___getcwd() gets enough
parameters as it is; please let's leave it for another time.

kib accepted this revision.Apr 21 2015, 5:43 PM

kib edited edge metadata.

This revision is now accepted and ready to land.Apr 21 2015, 5:43 PM

emaste added a subscriber: emaste.Apr 22 2015, 3:43 AM

dchagin accepted this revision.Apr 22 2015, 7:33 AM

dchagin added a reviewer: dchagin.

Closed by commit rS281882: Simplify linux_getcwd(), removing code that was longer used. (authored by trasz). · Explain WhyApr 23 2015, 8:41 AM

This revision was automatically updated to reflect the committed changes.