Skip to content
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.

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

OS_statvfs_t types do not match corresponding POSIX types #570

Closed
ghost opened this issue Aug 20, 2020 · 4 comments · Fixed by #654
Closed

OS_statvfs_t types do not match corresponding POSIX types #570

ghost opened this issue Aug 20, 2020 · 4 comments · Fixed by #654
Milestone

Comments

@ghost
Copy link

ghost commented Aug 20, 2020

When building for 64-bit, Wconversion shows issues with OS_statvfs_t's types. They don't match the types in struct statvfs on POSIX systems. It may be worth a discussion to change these types of just handle the conversions in the specific implementation.

@jphickey
Copy link
Contributor

The OS_statvfs_t is currently defined as:

typedef struct
{
    uint32 block_size;
    uint64 total_blocks;
    uint64 blocks_free;
} OS_statvfs_t;

I assume a conversion warning would have been in relation to the block_size element, as it is the only 32-bit value in this struct., and POSIX uses unsigned long Hard to imagine a file system which has a block size that exceeds 32 bits.

FWIW, my experience with -Wconversion is that while its interesting and worth looking at the things it complains about, it yields far too many false positives to be useful.

Would recommend just doing a typecast or something to squelch the warning, rather than change OSAL API. Unless there is an expectation of seeing a filesystem block size that exceeds 32 bits?

@jphickey
Copy link
Contributor

On further thought, the struct almost certainly has 32 bits of padding following that value anyway, so changing block_size to be a uint64 type would be pretty benign. If that fixes the warning, that's fine too - but note it may just shift the warning to later on when an app references this value, so maybe not a fix.

@ghost
Copy link
Author

ghost commented Aug 20, 2020

I"m fine with not changing it. Put the uint32 at the botttom of the struct to avoid padding . ;)

@skliper skliper linked a pull request Dec 9, 2020 that will close this issue
@skliper
Copy link
Contributor

skliper commented Dec 9, 2020

Fixed in #654 (confirmed no conversion warning currently on a 64 bit posix build.)

@skliper skliper closed this as completed Dec 9, 2020
@skliper skliper added this to the 6.0.0 milestone Dec 9, 2020
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants