[PATCH 2/7] df: Use statvfs instead of non-standard statfs

Ralf Friedl Ralf.Friedl at online.de
Mon Oct 23 09:12:08 UTC 2017


Eugene Rudoy wrote:
> just wanted to point out that besides making the code more portable
> the change suggested by James has another positive side effect. It
> eliminates the need for the uClibc related and as of now unreliable
> workaround added in the second hunk of e184a88 [1].
>
> Why unreliable? "statfs s"-variable is allocated on stack and is not
> initialized (=zeroed) at all, neither on the caller nor on the callee
> side. So the workaround works only in those cases where the stack (by
> chance) contain zeros (bytes corresponding to f_frsize would be
> enough).
>
> For the sake of completeness... The problem the code tries to
> workaround could (as of now) be observed only on mips. See mips
> specific libc/sysdeps/linux/mips/bits/statfs.h [2]. It contains
> f_frsize in all its statfs* structs but doesn't signal it by defining
> _STATFS_F_FRSIZE like e.g. libc/sysdeps/linux/common/bits/statfs.h [3]
> does. This in turn results in libc/misc/statfs/statfs64.c [4] ignores
> the field and doesn't copy it (like the comment to the workaround
> states).
>
> statvfs doesn't have this problem, it always sets f_frsize (s. [5]).
I just want to point out that while uClibc statvfs does always set 
f_frsize, currently on MIPS at least it doesn't use the value returned 
by the kernel, instead it copies the value from f_bsize, thus negating 
commit 
https://git.busybox.net/busybox/commit/coreutils/df.c?id=8f4faa1e3db91fc7b50d633e6f9b2f04bf978bb2
Additionally, the uClibc implementation always calls stat() on the 
supplied argument and for older kernel at least reads /proc/mounts, 
calls stat() on the entries and tries to determine the mount flags that 
statvfs returns but df doesn't use.

What about using statfs and adding a define to platform.h to determine 
whether the value returned by statfs contains f_frsize?
To support the buggy implementation in uClic, f_frsize must be set to 
zero before the call and tested later, because it is left unmodified.
> [1] https://git.busybox.net/busybox/commit/coreutils/df.c?id=e184a883567ee3fd735644416e4bd683f1894ac5
> [2] https://cgit.openadk.org/cgi/cgit/uclibc-ng.git/tree/libc/sysdeps/linux/mips/bits/statfs.h
> [3] https://cgit.openadk.org/cgi/cgit/uclibc-ng.git/tree/libc/sysdeps/linux/common/bits/statfs.h#n66
> [4] https://cgit.openadk.org/cgi/cgit/uclibc-ng.git/tree/libc/misc/statfs/statfs64.c?id=7cfd2112acbc7a9dfcd9f8a23e550672934a2545#n48
> [5] https://cgit.openadk.org/cgi/cgit/uclibc-ng.git/tree/libc/misc/statfs/internal_statvfs.c#n24
>
> On Sat, Oct 7, 2017 at 7:53 PM, James Clarke <jrtc27 at jrtc27.com> wrote:
>> Platforms differ on what their implementations of statfs include.
>> Importantly, FreeBSD's does not include a f_frsize member inside struct
>> statfs. However, statvfs is specified by POSIX and includes everything
>> we need, so we can just use that instead.



More information about the busybox mailing list