Page MenuHomeFreeBSD

ino64
ClosedPublic

Authored by kib on Apr 20 2017, 5:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 24 2024, 12:40 AM
Unknown Object (File)
Feb 22 2024, 4:03 AM
Unknown Object (File)
Feb 16 2024, 7:47 AM
Unknown Object (File)
Feb 16 2024, 7:47 AM
Unknown Object (File)
Feb 16 2024, 7:47 AM
Unknown Object (File)
Feb 16 2024, 7:38 AM
Unknown Object (File)
Feb 16 2024, 7:37 AM
Unknown Object (File)
Feb 16 2024, 4:24 AM

Details

Summary

See post-patch /testing.txt for explanation.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 8816
Build 9187: CI src buildJenkins

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Thank you very much for working on this.

I took a very quick pass over some spots. I understand that some of this is copying old compat code, but I was wondering if now was the right time to fix stylistic issues and other nits before duplicating more copies of the same code around.

TODO.ino64
1

This probably shouldn't be committed.

abi.sh
1

Hmmm... this is an interesting script. What does it do?

contrib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc
226–227

Should the "hacks" be upstreamed? They seem more preferable over calling lstat, stat, etc.

contrib/openbsm/libbsm/bsm_wrappers.c
267

Would rwatson care about these changes?

lib/libc/gen/devname-compat11.c
43

Why int instead of size_t?

lib/libc/gen/fts-compat.c
746

dirp != NULL?

lib/libc/gen/fts-compat11.c
124

Please don't use nitems (it masks the nitems(..) macro in sys/param.h).

182

compar != NULL?

216

Unnecessary spaces between O_RDONLY, |, and O_CLOEXEC?

243

cp[1] != '\0'

263

.. != NULL

275

The if-statement is unnecessary per-POSIX.

311

style(9): char *t comes before int instr?

638

Move this down to sort per style(9)?

650

Remove the #ifdef since this feature has been present since 1997?

$ git blame include/fts.h | grep FTS_WHITEOUT
53b81178e2d0a (peter 1997-03-11 11:11:37 +0000 57) #define FTS_WHITEOUT 0x080 /* return whiteout information */

651–654
oflag = DTF_NODUP | DTR_REWIND;
if (ISSET(FTS_WHITEOUT))
    oflag |= DTR_HIDEW;
```?
686

Remove deadcode?

796

DT_DIR has been around since 1994 -- this should probably be removed too:

$ git blame sys/sys/dirent.h | grep DT_DIR
8fb65ce818b3e (rgrimes 1994-05-24 10:09:53 +0000 70) #define DT_DIR 4

882–883

style(9): sort by type?

1109

#undef ADJUST?

1151

oerrno vs saved_errno is a bit of an odd convention.

lib/libc/gen/fts-compat11.h
33

_FTS_COMPAT11_H?

45

fts_compare?

lib/libc/gen/ftw-compat11.c
38

This should be maxfds to match the manpage. opengroup.org calls it ndirs though, so maybe that should be the code standard of sorts.

90

sverrno vs saved_errno vs oerrno is inconsistent.

lib/libc/gen/getmntinfo-compat11.c
51

style(9): sort values?

60–64

Could this use reallocf or reallocarray instead?

lib/libc/gen/glob-compat11.c
189

Move the memset out in both scenarios to reduce duplication?

229

Whitespace issue here and the line after next?

939–940

style(9): sort?

1021

I've seen this sprinkled around this file quite a bit. It would be nice if there was a better reference to the comment of interest since initial comment # 2 is vague.

lib/libc/gen/scandir-compat11.c
55

SCANDIR_B?

lib/libc/sys/getdirentries.2
35

Bump .Dd at commit time?

lib/libc/sys/lstat.c
33–34

Sort sys/stat.h before sys/syscall.h?

lib/libc/sys/mknod.c
34

Sort sys/stat.h before sys/syscall.h?

lib/libc/sys/stat.c
32

Why sys/fcntl.h instead of fcntl.h? fcntl.h is more portable.

34

Sort sys/stat.h before sys/syscall.h?

lib/libc/sys/statfs.2
70

Should this be bumped to a more recent date?

lib/libprocstat/libprocstat_compat.c
33–34

Sort headers?

share/man/man5/dir.5
109–114

This seems like it could be split off of this CR.

Also: should MAXNAMLEN be wrapped if a #ifndef MAXNAMLEN?

136

Could this be split off of the CR and committed separately (well, not the dp/namlen part, but the rest)?

sys/compat/freebsd32/freebsd32_misc.c
1861

Why COMPAT_43?

1898

#ifdef?

sys/compat/linux/linux_file.c
1093–1094

This should arguably be a different constant because Linux might have different values from FreeBSD...

sys/fs/nfsserver/nfs_nfsdport.c
2029–2039

I'm confused. Why is the VFS_VGET only being done for zfs now?

sys/kern/kern_descrip.c
1307

#ifdef?

sys/kern/vfs_syscalls.c
646–647

Could this be committed separately?

3944

Could this be tunable for folks who have potentially large directories (maybe not in the compat syscall, but maybe in the non-compat one)?

sys/nlm/nlm_advlock.c
221

Why XXXKIB?

sys/sys/dirent.h
119–122

Is this a bit of a rehash of other headers?

sys/sys/mount.h
69

Bump the version number?

testing.txt
1

Remove file?

usr.bin/kdump/kdump.c
935

Why remove this? Should it be committed separately?

emaste added inline comments.
TODO.ino64
1

Indeed; this Phabricator review is a snapshot of the diff to the GitHub branch https://github.com/FreeBSDFoundation/freebsd/tree/ino64, including some text files which won't be committed.

abi.sh
1

This is a wrapper around abi-compliance-checker https://www.freshports.org/devel/abi-compliance-checker/ to compare ABIs of the upstream & ino64 branches. It could be made more general and placed in tools/ but for now it's a one-off for the project branch.

contrib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc
226–227

fstatat has been around since FreeBSD 8 so there should be no trouble using it universally and upstreaming this. @dim?

contrib/openbsm/libbsm/bsm_wrappers.c
267
lib/libc/gen/devname-compat11.c
43

because that's the type in FreeBSD 11's devname_r.

lib/libc/gen/fts-compat.c
746

yeah, style(9) bug exists in old version but if the line is going to be changed anyway might as well address it

lib/libc/gen/fts-compat11.h
45

in the great UNIX tradition of leaving off trailing es this has been spelled fts_compar since the beginning of FreeBSD.

lib/libc/sys/statfs.2
70

Although it's really just an opaque value, might as well use a recent (commit?) date.

sys/nlm/nlm_advlock.c
221

In the style of XXXRW etc.

testing.txt
1

Yes this one is also not for inclusion in the commit

I deliberately refuse to make any gratuitous style changes in the copied files. They have to be kept in sync with the master copies.

I did not found a sane way to avoid the copying and not make the non-compat code the ugly mess. If anybody have sensible ideas how to achieve that, I am more than happy to implement the proposal.

TODO.ino64
1

If you really looked at the files, esp. at the preapply.sh, you would see how all this is handled.

contrib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc
226–227

It is not about 'preferable'. The stat(2) syscall is removed in the patch, otherwise I would not touch llvm code with a long pole.

wblock added inline comments.
sys/sys/dirent.h
55

s/a 8/an 8/

testing.txt
53

s/depended/dependant/
or possibly "depending". The "in general" at the end is kind of confusing.

We handled kinfo sysctl MIBs, but, in general, other MIBs which report structures
depending on the changed type are not handled.
kib marked an inline comment as done.May 5 2017, 3:41 PM
kib added inline comments.
testing.txt
53

The word meant to inform that we provide no general mechanism to handle sysctls which return structures with changed layout. Moving 'in general' to the place you proposed changes the meaning.

testing.txt
53

Ah, that helps. Well, see what you think:

We handled kinfo sysctl MIBs, but there is no general mechanism to handle other
sysctl MIBS which return structures where the layout has changed.
kib marked an inline comment as done.May 5 2017, 5:27 PM

Update to the current patch. Most important change is probably a space to extend time_t in struct stat on i386.

This revision was automatically updated to reflect the committed changes.
testing.txt
53

s/depended/depending/