0x09 / hfsfuse

FUSE driver for HFS+ filesystems
Other
77 stars 13 forks source link

Failure in ARM platform - solved. #20

Closed ttsiodras closed 3 years ago

ttsiodras commented 3 years ago

Hi. Thank you for creating hfsfuse, it is really useful - but I wanted to share an issue I had and how I solved it, when I needed to compile the code under an ARM board (a Rock-PI-S running Armbian).

Using make led to this:

(master)$ make |& head
cc -Wall -Wextra -pedantic -Wno-gnu-zero-variadic-macro-arguments -Wno-unused-parameter -Wno-missing-field-initializers -Wno-missing-braces -DFUSE_USE_VERSION=28 -I/home/ttsiod/GitHub/hfsfuse/lib/libhfsuser -I/home/ttsiod/GitHub/hfsfuse/lib/libhfs -I/home/ttsiod/GitHub/hfsfuse/lib/ublio -I/home/ttsiod/GitHub/hfsfuse/lib/utf8proc -D_FILE_OFFSET_BITS=64 -O3 -std=gnu11  -DHFSFUSE_VERSION_STRING=\"0.120-a089632\" -c -o src/hfsfuse.o src/hfsfuse.c
In file included from /home/ttsiod/GitHub/hfsfuse/lib/libhfsuser/hfsuser.h:29,
                 from src/hfsfuse.c:25:
/usr/include/aarch64-linux-gnu/sys/stat.h:99:14: error: expected ‘;’ before ‘struct’
 __BEGIN_DECLS
              ^
              ;

...which looked weird - why does this code work in other platforms?

Looking things up, I realised I had to #include "features.h" - so I did...

--- a/lib/libhfsuser/hfsuser.h
+++ b/lib/libhfsuser/hfsuser.h
@@ -25,6 +25,7 @@
 #ifndef HFSUSER_H
 #define HFSUSER_H

+#include <features.h>
 #include <sys/stat.h>

 #include "libhfs.h"

...only to find that the compilation still fails with the same error. After some more head-scratching, and adding "-save-temps" to see exactly what happens during preprocessing, I realized why this patch failed...

There's a "features.h" in our own code - that somehow takes precedence! (even though I used angle brackets - that should prioritize the system-provided header instead of the one under lib/libhfsuser/features.h )

So, as a quick hack, I changed the #include to ...

#include </usr/include/features.h>

...and after doing this patch in one more place, everything worked perfectly.

I am not sure how this can be addressed. I thought of preparing a PR where I rename this code's "features.h" to a filename that doesn't clash with mainstream OSes default header files, but realised you may very well object to this - so I instead opened this issue. For completeness, these are the two patches I had to make for the code to compile:

(master)$ git diff 
diff --git a/lib/libhfsuser/features.c b/lib/libhfsuser/features.c
index 53aabfb..03f55a7 100644
--- a/lib/libhfsuser/features.c
+++ b/lib/libhfsuser/features.c
@@ -22,6 +22,7 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */

+#include "/usr/include/features.h"
 #include "features.h"
 #include "hfsuser.h"

diff --git a/lib/libhfsuser/hfsuser.h b/lib/libhfsuser/hfsuser.h
index 532c49c..a9b1f32 100644
--- a/lib/libhfsuser/hfsuser.h
+++ b/lib/libhfsuser/hfsuser.h
@@ -25,6 +25,7 @@
 #ifndef HFSUSER_H
 #define HFSUSER_H

+#include </usr/include/features.h>
 #include <sys/stat.h>

 #include "libhfs.h"
0x09 commented 3 years ago

Hi @ttsiodras

Thanks, it looks like this was broken by https://github.com/0x09/hfsfuse/commit/8df0c23ffa111ffc0c4f5aed0d142fd8a8913530 which added libhfsuser (and others under lib/) to the include path in order to fix inconsistencies building with the bundled vs system utf8proc/ublio. But this also means that the couple of internal headers in those dirs are also added to the path, and since gcc/clang search locations added with -I before system ones, the effect is that when sys/stat.h includes <features.h> libhfsuser's features.h is being included instead.

Renaming the header isn't enough of a solution here because the project shouldn't really be organized in a way that requires the build system to do this. I think the correct thing here would be to just restructure libhfs and libhfsuser so that only their public headers (ones that would be installed with make install-lib) are added to the search path and deal with ublio/utf8proc separately. I'll push up a fix a bit later.

ttsiodras commented 3 years ago

I'll push up a fix a bit later.

Excellent. Ping me and I will test it.

0x09 commented 3 years ago

@ttsiodras I decided to go with a much simpler solution of using -iquote for these includes since this fits better with how the libs are being used internally. Confirmed https://github.com/0x09/hfsfuse/commit/32a77edb422443ed82a91f6d06e0f5966d221c7f resolves this specific issue on other systems with glibc, but should stop the issue of polluting the include path in general.

ttsiodras commented 3 years ago

I just tested it - works great.

Thank you!

0x09 commented 3 years ago

@ttsiodras great, thanks for reporting this.