Skip to content

Conversation

@guibou
Copy link

@guibou guibou commented Sep 25, 2015

(This is a request for discussion, there is, at least, one point which must be handled before merging this code)

The system stat function returns blocksize and blocks count for the file, which are not handled by current implementation. I added the accessor for both fields.

Discussion

I think the new types aliases:

type BlockSize = CSsize
type BlocksCount = CSsize

Should be placed somewhere else (in base ?) and I'm not really sure that's the correct types.

@argiopetech
Copy link
Contributor

Thanks for your work on this, guibou.

Regarding the aliases, those would go in System.Posix.Types. That will require a pull request against base. I'm honestly not sure what the historical context is for those definitions to be in base rather than unix, but I imagine there's a good reason for it. We'll need to bump the lower bound on the base version dependency in unix.cabal to match the upcoming base release in order to ensure those types are available.

@argiopetech
Copy link
Contributor

Oh, and you are correct about the types needing to be changed. CSsize is equivalent to size_t, but the current POSIX spec specifies blkcount_t and blksize_t for st_blksize and st_blocks (respectively). Those will need to be added as new types using the INTEGRAL_TYPE or INTEGRAL_TYPE_WITH_CTYPE macros.

@DanielG
Copy link
Contributor

DanielG commented Nov 2, 2016

I just created a trac ticked for this and patches to both base and unix are forthcoming: https://ghc.haskell.org/trac/ghc/ticket/12795

@guibou
Copy link
Author

guibou commented Jul 23, 2018

@argiopetech Thanks to @DanielG changes to base, I can now update this PR without the ugly type aliases.

(Yes, two years had passed ;)

@DanielG
Copy link
Contributor

DanielG commented Jul 24, 2018

Probably should have mentioned this before but I also have a PR open for this: #78. It handles platforms where st_blksize doesn't exist which this PR doesn't.

@guibou
Copy link
Author

guibou commented Jul 24, 2018

@DanielG good point. I'm closing in favor of #78

@guibou guibou closed this Jul 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants