OS_fsBlocksFree returns a 32-bit value, but OS_statvfs_t.blocks_free is 64 bit

[ghost]

OS_fsBlocksFree returns a 32-bit value, but OS_statvfs_t.blocks_free is 64 bit.

[jphickey]

Similar to #570 -- while this return type (int32) is small for modern storages and filesystems, it was seen in previous reviews as being "good enough" for what you'd likely find on a flight system. Particularly because its returning blocks, not bytes. It's also overloaded with an error code on failure, which needs to be the int32 type.

I wouldn't recommend changing the return type for that reason. Would recommend to deprecate this API rather than change it, if it is really a problem. New code should just use the existing OS_statfs() call anyway, which returns everything.

[ghost]

I'm fine with not changing it. I just wanted to bring it to the group's attention.

[skliper]

Recommending deprecate in favor of using OS_statfs

[skliper]

There is no OS_statfs. If OS_fsBlocksFree is removed, seems like there's no replacement (other than using statvfs directly).

[skliper]

@jphickey could you clarify the "New code should just use the existing OS_statfs()" recommendation?

[jphickey]

Hmm, you are correct, it looks like we do not expose the OS_FileSysStatVolume_Impl info via a public API currently.

I can easily implement one as long as folks are in agreement that it is a reasonable solution. The structure currently is defined as:

typedef struct
{
    size_t            block_size;
    osal_blockcount_t total_blocks;
    osal_blockcount_t blocks_free;
} OS_statvfs_t;

Prototype would be:

int32 OS_FileSysStatVolume(const char *path, OS_statvfs_t *statbuf);

Filesystem code has traditionally used strings/pathnames to identify objects so this is consistent with other filesystem calls, as opposed to using an osal_id_t to identify the filesystem (which would be more like the rest of OSAL).

[jphickey]

After this, we would deprecate OS_fsBlocksFree() and OS_fsBytesFree(). For the latter, applications would simply multiply blocks_free * block_size on their own.

[skliper]

I'm good with your suggestions above to resolve this issue.