Fix NFS detection on the BSDs #411
Conversation
seanm
commented
Oct 16, 2023
seanm
commented
- on the BSDs, look at the f_fstypename field to determine file system type, as this is the documented way to do that. The sqlite codebase also does so for macOS notably.
- removed dead code in the Windows case
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request |
There was a problem hiding this comment.
Seems fine. Though I'll note that it looks like Windows does support NFS: https://learn.microsoft.com/en-us/windows-server/storage/nfs/nfs-overview
Something for the future where someone cares enough about it to add detection here.
- on the BSDs, look at the f_fstypename field to determine file system type, as this is the documented way to do that. The sqlite codebase also does so for macOS notably. - removed dead code in the Windows case
seanm
force-pushed
the
statfs-improvements
branch
from
c64b639 to
d0c14d7
Compare
|
I have read the CLA Document and I hereby sign the CLA |
| return (0 == ::strcmp("nfs", stat_fs.f_fstypename)); | ||
| #else | ||
| /* linux statfs defines that 0x6969 is NFS filesystem */ | ||
| return (stat_fs.f_type == 0x6969); |
There was a problem hiding this comment.
I would prefer that the #define NFS_FS be used here. Many static analyzers complain about magic numbers. I realize that the define would be used in exactly one place and that there are many other magic numbers throughout the code base, but ...
Agreed. There seem to be many more uses on Windows systems, so code should be added instead of just "punting..." |
