Details

Reviewers

emaste
markj
kevans
obiwac_gmail.com
imp

Group Reviewers

manpages

Commits

rGa294e679738b: Document scandirat(3)
rG9fb8e8eede5b: libc: add scandirat(3)
rGcb6e97f4dae6: libc: split scandir() into scandir_dirp() and proper scandir()
rGea448a0a43c5: Link scandir_b(3) to scandir(3)
rGaa5e19a9b3f3: scandir.3: Use .Fo instead of .Fn for long args

Summary

Seen in glibc

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Aug 23 2022, 5:04 AM

Herald added a subscriber: imp. · View Herald TranscriptAug 23 2022, 5:04 AM

kib requested review of this revision.Aug 23 2022, 5:04 AM

kib added a reviewer: manpages.

LGTM for the man page parts.

emaste accepted this revision.Aug 23 2022, 1:34 PM

emaste added inline comments.

include/dirent.h
135 ↗	(On Diff #109700)	any reason the first argument [only] is named?

This revision is now accepted and ready to land.Aug 23 2022, 1:34 PM

kib marked an inline comment as done.Aug 23 2022, 1:35 PM

kib added inline comments.

include/dirent.h
135 ↗	(On Diff #109700)	No, it was copy/paste. Thanks for noting.

Do not name dirfd in scandirat(3) prototype.

This revision now requires review to proceed.Aug 23 2022, 1:37 PM

emaste accepted this revision.Aug 23 2022, 1:38 PM

This revision is now accepted and ready to land.Aug 23 2022, 1:38 PM

Looks good. Maybe add a 'tidy up a few scandir_b related issues while in the area' to the commit message?
Otherwise looks great.

lib/libc/gen/Makefile.inc
498 ↗	(On Diff #109721)	I'd mention adding this link in the commit since it was missing before (eg, it's otherwise unrelated to this change)

Looks fine to me aside from the manpage typos :)

lib/libc/gen/scandir.3
35	Shouldn't `scandirat` be added here too?
107	Missing "an" ("takes an additional `dirfd` argument")?
114

In D36301#824708, @imp wrote:

Looks good. Maybe add a 'tidy up a few scandir_b related issues while in the area' to the commit message?
Otherwise looks great.

This is the cumulative diff against the branch.
Per commit view is available at https://kib.kiev.ua/git/gitweb.cgi?p=deviant3.git;a=shortlog;h=refs/heads/scandirat

Man page fixes.
Add scandirat and scandir_b to .Nm's of scandir.3

This revision now requires review to proceed.Aug 24 2022, 3:15 AM

kevans accepted this revision.Aug 24 2022, 3:19 AM

This revision is now accepted and ready to land.Aug 24 2022, 3:19 AM

obiwac_gmail.com accepted this revision.Aug 24 2022, 2:11 PM

markj accepted this revision.Aug 24 2022, 3:43 PM

markj added inline comments.

lib/libc/gen/scandir.3
112
118	The wording implies that in the case where dirname is absolute, dirfd may be invalid. But, that's not true right? So in the description of the absolute case, perhaps add, "The .Fa dirfd argument is unused, but must be a valid file descriptor or the special value .Dv AT_FDCWD."
121

kib marked 2 inline comments as done.Aug 24 2022, 5:33 PM

kib added inline comments.

lib/libc/gen/scandir.3
118	Hm, I thought that dirfd can be invalid. And both from my reading of namei_setup(), and actual testing, dirfd is ignored when first character of path is '/': openat(-10000004,"/etc/passwd",O_RDONLY,00) = 3 (0x3)