Skip to content

Add accessors for st_blocks and st_blksize #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Add accessors for st_blocks and st_blksize #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions System/Posix/Files.hsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ module System.Posix.Files (
isBlockDevice, isCharacterDevice, isNamedPipe, isRegularFile,
isDirectory, isSymbolicLink, isSocket,

fileBlockSize,
fileBlocks,

-- * Creation
createNamedPipe,
createDevice,
Expand Down
3 changes: 3 additions & 0 deletions System/Posix/Files/ByteString.hsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ module System.Posix.Files.ByteString (
isBlockDevice, isCharacterDevice, isNamedPipe, isRegularFile,
isDirectory, isSymbolicLink, isSocket,

fileBlockSize,
fileBlocks,

-- * Creation
createNamedPipe,
createDevice,
Expand Down
24 changes: 24 additions & 0 deletions System/Posix/Files/Common.hsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ module System.Posix.Files.Common (
isBlockDevice, isCharacterDevice, isNamedPipe, isRegularFile,
isDirectory, isSymbolicLink, isSocket,

fileBlockSize,
fileBlocks,

-- * Setting file sizes
setFdSize,

Expand Down Expand Up @@ -255,6 +258,14 @@ specialDeviceID :: FileStatus -> DeviceID
-- | Size of the file in bytes. If this file is a symbolic link the size is
-- the length of the pathname it contains.
fileSize :: FileStatus -> FileOffset
-- | Number of blocks allocated for this file, in units of
-- 512-bytes. Returns @Nothing@ if @st_blocks@ is not supported on this
-- platform.
fileBlocks :: FileStatus -> Maybe CBlkCnt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The types CBlkCnt and CBlkSize below don't appear to be defined with GHC <= 8.0 breaking CI.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed these were introduced conditionally in GHC 8.2 and this PR can probably use the corresponding macros to detect their presence. The new feature can then only be offered if the stat structure has the requisite fields (already done) AND the types are available.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. Not sure how we missed that before. I'll add some more GLASGOW_HASKELL version checking then.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This CPP guard is not sufficient. GHC 8.2 conditionally defines these types. So you need to:

#include "HsBaseConfig.h"

and use the same feature macros.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which means that the types might in fact be unsupported on some platforms even with GHC >= 8.2. However, when that's the case we could define them to be new types for Void in that case, and then the functions can return Maybe X -- X ~ Void, which can only be instantiated as Nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that, but that doesn't work because HsBaseConfig.h defines a bunch of conflicting defines like PACKAGE_NAME etc. which we also have in HsUnix.h.

I'm pretty sure that if the st_ fields exist the corresponding types must also be defined in the system headers so there shouldn't be any need to check for the types and struct fields separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then the functions can return Maybe X -- X ~ Void

I thought about that too, but then the API is conditional depending on platform factors again :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure that if the st_ fields exist the corresponding types must also be defined

Looks like these didn't always exist, at least according to https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_stat.h.html

Issue 5

[...]
The type of st_blksize is changed from long to blksize_t; the type of st_blocks is changed from long to blkcnt_t.

-- | Gives the preferred block size for efficient filesystem I/O in
-- bytes. Returns @Nothing@ if @st_blocksize@ is not supported on this
-- platform.
fileBlockSize :: FileStatus -> Maybe CBlkSize
-- | Time of last access.
accessTime :: FileStatus -> EpochTime
-- | Time of last access in sub-second resolution. Depends on the timestamp resolution of the
Expand Down Expand Up @@ -294,6 +305,19 @@ modificationTime (FileStatus stat) =
statusChangeTime (FileStatus stat) =
unsafePerformIO $ withForeignPtr stat $ (#peek struct stat, st_ctime)

#ifdef HAVE_STRUCT_STAT_ST_BLOCKS
fileBlocks (FileStatus stat) =
Just $ unsafePerformIO $ withForeignPtr stat $ (#peek struct stat, st_blocks)
#else
fileBlocks _ = Nothing
#endif
#ifdef HAVE_STRUCT_STAT_ST_BLKSIZE
fileBlockSize (FileStatus stat) =
Just $ unsafePerformIO $ withForeignPtr stat $ (#peek struct stat, st_blksize)
#else
fileBlockSize _ = Nothing
#endif

accessTimeHiRes (FileStatus stat) =
unsafePerformIO $ withForeignPtr stat $ \stat_ptr -> do
sec <- (#peek struct stat, st_atime) stat_ptr :: IO EpochTime
Expand Down
3 changes: 3 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ AC_CHECK_MEMBERS([struct stat.st_uatime])
AC_CHECK_MEMBERS([struct stat.st_umtime])
AC_CHECK_MEMBERS([struct stat.st_uctime])

AC_CHECK_MEMBERS([struct stat.st_blocks])
AC_CHECK_MEMBERS([struct stat.st_blksize])

AC_CHECK_MEMBER([struct passwd.pw_gecos], [], [AC_DEFINE([HAVE_NO_PASSWD_PW_GECOS],[],[Ignore the pw_gecos member of passwd where it does not exist])], [[#include <pwd.h>]])

# Functions for changing file timestamps
Expand Down