This is an archive of the discontinued LLVM Phabricator instance.

[Support] Provide linux/magic.h fallback for older kernels
ClosedPublic

Authored by mgorny on Feb 22 2017, 9:12 AM.

Details

Summary

The function for distinguishing local and remote files added in r295768
unconditionally uses linux/magic.h header to provide necessary
filesystem magic numbers. However, in kernel headers predating 2.6.18
the magic numbers are spread throughout multiple include files.
Furthermore, LLVM did not require kernel headers being installed so far.

To increase the portability across different versions of Linux kernel
and different Linux systems, add CMake header checks for linux/magic.h
and -- if it is missing -- the linux/nfs_fs.h and linux/smb.h headers
which contained the numbers previously.

Furthermore, since the numbers are static and the feature does not seem
critical enough to make LLVM require kernel headers at all, add fallback
constants for the case when none of the necessary headers is available.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Feb 22 2017, 9:12 AM
zturner accepted this revision.Feb 22 2017, 9:21 AM

Seems like a good fix, thanks for doing this.

This revision is now accepted and ready to land.Feb 22 2017, 9:21 AM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Apr 1 2018, 6:56 PM
thakis added inline comments.
llvm/trunk/lib/Support/Unix/Path.inc
79

The three HAVE_LINUX_ referenced here aren't listed in include/llvm/Config/config.h.cmake, so they never make it across from the cmake world to the C world -- they're never defined here. Since this also adds fallback code defining _SUPER_MAGIC things still work, but the cmake checks above are effectively dead code. Since we already have the fallbacks, maybe we should just delete the cmake checks instead of adding yet more stuff to config.h.cmake?

labath added inline comments.Apr 3 2018, 2:16 AM
llvm/trunk/lib/Support/Unix/Path.inc
79

sounds good to me. It's not like those numbers are ever going to change.