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 <[email protected]> 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.
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
