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.

Sign up for GitHub

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

Jump to bottom

Fix various portability issues #202

Merged
merged 7 commits into from
Sep 26, 2023
Merged

Fix various portability issues #202

merged 7 commits into from
Sep 26, 2023

Conversation

alexlarsson
Copy link
Collaborator

This should fix the issues pointed out in #200 (although I never tried to actually build on any other os).

@Kiskae
Copy link
Contributor

Kiskae commented Sep 25, 2023

After removing AC_FUNC_ALLOC/AC_FUNC_REALLOC from autoconf I was able to compile a static binary with these changes.

./result/bin/mkcomposefs: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, not stripped

Those autoconf statements redirect the code through a rpl_malloc function, which does to not exist in musl/some clang stdlibs.

Cannot confirm whether the darwin compilation builds at the moment.

Kiskae
Kiskae reviewed Sep 25, 2023
View reviewed changes
tests/gendir Show resolved Hide resolved
@alexlarsson
tests/gendir: Don't exit with exception if xattr not supported
b23247f 
If setxattrs fail we just don't test xattrs. Not great, but also not
a huge issue.

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
Copy link
Collaborator Author

rpl_malloc is meant to be supplied manually by the code if, but we don't do this atm. I don't think we depend on the gnu malloc(0) behaviour tho, so lets just drop these.

@Kiskae
Copy link
Contributor

Kiskae commented Sep 25, 2023

  • test-random-fuse works on filesystems without xattrs
  • static linking with musl works
  • compilation on darwin does not work
    • it looks like apple simply does not supply endian.h, you need to provide your own wrapper to libkern/OSByteOrder.h
  • compilation on clang fails with cfs-fuse.c:57:19: error: format string is not a string literal
    • disabling fuse simply moves the error to mountcomposefs.c:46:19: error: format string is not a string literal

@alexlarsson
Copy link
Collaborator Author 

* compilation on darwin does not work
 
  * it looks like apple simply does not supply `endian.h`, you need to provide your own wrapper to `libkern/OSByteOrder.h`

This will have to be done by someone with darwin then.

* compilation on clang fails with `cfs-fuse.c:57:19: error: format string is not a string literal `
  
  * disabling fuse simply moves the error to `mountcomposefs.c:46:19: error: format string is not a string literal`

Fixed

Kiskae
Kiskae approved these changes Sep 25, 2023
View reviewed changes
Copy link
Contributor

@Kiskae Kiskae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm that both builds with musl or clang compile successfully.

Although it might be an idea to add this to libcomposefs/Makefile-lib.am so the current version gets set as SONAME:

libcomposefs_la_LDFLAGS = -release @VERSION@

@alexlarsson
Copy link
Collaborator Author

Can confirm that both builds with musl or clang compile successfully.

Although it might be an idea to add this to libcomposefs/Makefile-lib.am so the current version gets set as SONAME:

libcomposefs_la_LDFLAGS = -release @VERSION@

The version number of the release is more of a human consumable moniker. We don't want to tread it as an actual soname. For example, we want to be able to go to 1.0.0 without breaking the library ABI.

We should probably have some minor soname changes tho..

cgwalters
cgwalters previously requested changes Sep 25, 2023
View reviewed changes
tools/composefs-from-json.c Outdated Show resolved Hide resolved
@alexlarsson
tools: Don't user error.h for better portability
641e5d3 
This is a glibc specific extension. We were already using the BSD
alternative err()/errx() in some places, and that seems more widely
suported, so lets just use that everywhere.

Most of the conversions are trivial:

 * error(EXIT_FAILURE, errno, => err(EXIT_FAILURE,
 * error(EXIT_FAILURE, 0, => errx(EXIT_FAILURE,
 * error(0, 0, => fprintf(stderr,
 * error(0, errno, => perror(

Although in some places we had to set errno to be able to use err().

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
lcfs-verity: Fix name for data_size member of fsverity_descriptor
28bc517 
This member should be little endian, the matching kernel struct has:

   __le64 data_size;       /* size of file the Merkle tree is built over */

So, rename from data_size_be to data_size_le.

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
read-file.c: Include config.h
e8f2a7d 
Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
Use autoconf to look for the right endian.h header.
f2309fe 
Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
configure: Drop AC_FUNC_MALLOC/REALLOC as we don't use them
dd13da7 
To use these you should define your own rpm_malloc, and rpl_realloc, which
we don't. Also, we're not relying on zero allocations anyway, so we don't
need these.

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
Drop local printexit() helper in favour of errx()
ae3a021 
Signed-off-by: Alexander Larsson <[email protected]>
giuseppe
giuseppe approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alexlarsson alexlarsson merged commit a95115b into containers:main Sep 26, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kiskae Kiskae Kiskae approved these changes

@giuseppe giuseppe giuseppe approved these changes

@cgwalters cgwalters Awaiting requested review from cgwalters

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alexlarsson @Kiskae @giuseppe @cgwalters