Skip to content
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

Fix 32 bit compile on Fedora 40 #126

Merged
merged 1 commit into from
Feb 18, 2024
Merged

Fix 32 bit compile on Fedora 40 #126

merged 1 commit into from
Feb 18, 2024

Conversation

DrDaveD
Copy link
Collaborator

@DrDaveD DrDaveD commented Feb 15, 2024

This fixes 32 bit compiles on Fedora 40. Without it, this compiler error shows:

+ make squashfuse_ll
  CC       squashfuse_ll-ll_main.o
ll_main.c: In function ‘main’:
ll_main.c:164:41: error: assignment to ‘void (*)(struct fuse_req *, fuse_ino_t,  uint64_t)’ {aka ‘void (*)(struct fuse_req *, long long unsigned int,  long long unsigned int)’} from incompatible pointer type ‘void (*)(struct fuse_req *, fuse_ino_t,  long unsigned int)’ {aka ‘void (*)(struct fuse_req *, long long unsigned int,  long unsigned int)’} [-Wincompatible-pointer-types]
  164 |         sqfs_ll_ops.forget              = sqfs_ll_op_forget;
      |                                         ^
make: *** [Makefile:1373: squashfuse_ll-ll_main.o] Error 1

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented Feb 15, 2024

@vasi MacOS appears to be requiring that third parameter to be long instead of unit64_t. Do you have any suggestions for me?

The freebsd check also failed, but for a different reason: Not enough compute credits to prioritize tasks!. I don't know what to do about that either.

@vasi
Copy link
Owner

vasi commented Feb 16, 2024

For MacOS vs Linux parameters, I guess we should add a check in configure, and then use ifdefs. See eg: the existing checks here.

For the FreeBSD build, I don't think that's the real error--underneath it, we have The resource 'projects/freebsd-org-cloud-dev/global/images/family/freebsd-12-3' was not found. Looks like Cirrus only supports FreeBSD 13 and higher now, we should try rebasing onto 14.0, the newest full release.

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented Feb 16, 2024

Thanks for the helpful hints.

The freebsd issue was fixed in #127, and then this branch was rebased.

I added the configure check and ifdefs, please review.

@DrDaveD DrDaveD requested a review from vasi February 16, 2024 18:52
@vasi
Copy link
Owner

vasi commented Feb 17, 2024

This looks good, but can we reverse the check? IE: Let's assume the standard libfuse arguments (uint64_t) are normal, and that needing something like "unsigned long" is the thing we're testing for.

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented Feb 17, 2024

This looks good, but can we reverse the check? IE: Let's assume the standard libfuse arguments (uint64_t) are normal, and that needing something like "unsigned long" is the thing we're testing for.

I could, but that means in most cases it will use unsigned long. Is that what you want? Only 32-bit does not work with unsigned long. I thought it would be better to use uint64_t in most places and only switch to unsigned long on Mac where uint64_t doesn't work.

@vasi
Copy link
Owner

vasi commented Feb 18, 2024

Oh I see, because many systems have compatible unsigned long and uint64_t types? So then it would look like unsigned long worked, and we'd use that. Hmm.

I guess in a perfect world, we'd check if one type worked, then check the other, and error if none do. But that does seem like too much effort!

@vasi vasi merged commit dca5492 into vasi:master Feb 18, 2024
7 checks passed
@DrDaveD DrDaveD deleted the fix-32bit branch February 19, 2024 14:45
@Biswa96
Copy link

Biswa96 commented Feb 21, 2024

The compiler error still happens with version 0.5.1 and clang.

squashfuse/src/ll_main.c:164:22: error: incompatible function pointer types assigning to 'void (*)(fuse_req_t, fuse_ino_t, unsigned long)' (aka 'void (*)(struct fuse_req *, unsigned long, unsigned long)') from 'void (fuse_req_t, fuse_ino_t, uint64_t)' (aka 'void (struct fuse_req *, unsigned long, unsigned long long)') [-Wincompatible-function-pointer-types]
        sqfs_ll_ops.forget              = sqfs_ll_op_forget;
                                        ^ ~~~~~~~~~~~~~~~~~

Full build log can be found here termux/termux-packages#19304

@vasi
Copy link
Owner

vasi commented Feb 21, 2024

Hmm, we see the message:

checking for 64_t third argument to fuse ll forget op... no

Unfortunately it doesn't look like your build artifacts include config.log or config.h, so it's hard to see what happened next. Are you able to reproduce manually?

@Biswa96
Copy link

Biswa96 commented Feb 21, 2024

Are you able to reproduce manually?

Yes, I can reproduce the compiler error. Do you need any specific information?

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented Feb 21, 2024

Yes, please look for what the compile error was for that 64_t check in config.log.

Is this a new error that didn't happen with 0.5.0? On what host operating system version, and with 64-bit or 32-bit? Oh, some of that information can be gleaned from the termux link you gave, but still, it would be better if you created a new issue with all the details and refer back to this issue.

@vasi
Copy link
Owner

vasi commented Feb 23, 2024

Looks like the problem was the tag prefix, Biswa96's branch is building now.

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.

3 participants