Skip to content

Add statvfs to Linux LibC #1029

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

Closed
wants to merge 2 commits into from
Closed

Conversation

dbwiddis
Copy link
Contributor

Iterating mounts may be overkill, could probably just test /...

public int[] _f_unused = new int[PADDING_SIZE]; // 0 on 64-bit systems
public NativeLong fsMountFlags;
public NativeLong fsMaxFilenameLength;
public int[] _fSpare = new int[6];
Copy link
Member

Choose a reason for hiding this comment

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

Could you please describe where the C structure is from? The linux manpage has this to say:

           struct statvfs {
               unsigned long  f_bsize;    /* Filesystem block size */
               unsigned long  f_frsize;   /* Fragment size */
               fsblkcnt_t     f_blocks;   /* Size of fs in f_frsize units */
               fsblkcnt_t     f_bfree;    /* Number of free blocks */
               fsblkcnt_t     f_bavail;   /* Number of free blocks for
                                             unprivileged users */
               fsfilcnt_t     f_files;    /* Number of inodes */
               fsfilcnt_t     f_ffree;    /* Number of free inodes */
               fsfilcnt_t     f_favail;   /* Number of free inodes for
                                             unprivileged users */
               unsigned long  f_fsid;     /* Filesystem ID */
               unsigned long  f_flag;     /* Mount flags */
               unsigned long  f_namemax;  /* Maximum filename length */
           };.
       // Here  the types fsblkcnt_t and fsfilcnt_t are defined in <sys/types.h>.
       // Both used to be unsigned long

This would indicate, that a mapping with all elements mapped as NativeLong and no padding would be correct. I'm also wondering about the ints used as padding. Are you sure that are not byte[]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The manpage defines the structure "approximately as follows". The header of sys/statvfs.h imports from bits/statvfs.h where the structure is defined. The relevant padding is a single 32-bit int:

#ifdef _STATVFSBUF_F_UNUSED
  int __f_unused;
#endif

That conditional define is based on having 32-bit wordsize:

#if (__WORDSIZE == 32 \
    && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
  #define _STATVFSBUF_F_UNUSED
#endif

I've tested this both on 32-bit and 64-bit Linux VMs (Ubuntu 14 out-of-the box) and got sane values (255) for f_namemax on both systems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Related but not relevant to this PR, the statvfs manpage on FreeBSD is rather amusing.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation. I should really now better to check the headers (I routinely pointed people to base windows bindings not on MSDN doc, but on the headers ...).

For _f_unused please bind it as a plain int. The field logic (taking it out of the field to be considered) still works. Then the padding calculation can be removed also.

For the structure names - I see why you named them how they are named. One thing you might want to consider is, that googling for the structures "header" names is easier when the original name is used. You could move the "speaking" names into javadoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. My thought with the array was hoping for future zero-length array compatibility. :)

@matthiasblaesing
Copy link
Member

Thank you. I merged the change via c2c3c09. The things I changed prior to merging:

  • the unittests failed because the usable space was incorrectly calculated: it was implemented as assertEquals(f.getUsableSpace(), vfs.f_bfree.longValue() * vfs.f_bsize.longValue()); and I updated it to assertEquals(f.getUsableSpace(), vfs.f_bavail.longValue() * vfs.f_bsize.longValue());
  • the unittests failed also because the field list was missing f_bsize and contained _f_spare (trailing space)
  • I updated the version of the next release to 5.1.0 as it will be now a feature release

@dbwiddis
Copy link
Contributor Author

That'll teach me to think "oh I'm just using the refactoring these names, nothing can go wrong." At some point I got out of sync and thought I fixed it. Guess not.

@dbwiddis dbwiddis deleted the statvfs branch February 4, 2019 17:25
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.

2 participants