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

Update project to JDK 22 and JEP 454 #61

Merged
merged 8 commits into from
Mar 21, 2024
Merged

Conversation

overheadhunter
Copy link
Member

@overheadhunter overheadhunter commented Feb 22, 2024

Updates this project to the new (and final) FFM API defined in JEP 454

Summary by CodeRabbit

  • New Features
    • Updated Java version to 22 for enhanced performance and compatibility.
    • Enhanced Java bindings for FUSE with the new Foreign Function & Memory API.
  • Refactor
    • Simplified FUSE operation handling and memory management across various components.
  • Documentation
    • Updated README to reflect the requirement of JDK 22 due to new Java bindings.

Copy link

coderabbitai bot commented Feb 22, 2024

Walkthrough

The project has transitioned to Java version 22, including updates in GitHub Actions workflows and project configurations. The --enable-preview flag was removed from various run configurations, reflecting stabilization in the JDK features used. Significant changes were made to the FUSE operation handling and memory management across different modules, aligning with the JDK 22's Foreign Function & Memory API requirements. This update enhances compatibility, performance, and code maintainability.

Changes

Files Change Summary
.github/workflows/build.yml, .github/workflows/publish-central.yml, .github/workflows/publish-github.yml Updated Java version from 21 to 22 in workflows for building, publishing to central repository, and publishing on GitHub
.idea/misc.xml Updated Java SDK version from 21 to 22 in project configuration
.idea/runConfigurations/... Removed --enable-preview flag from VM_PARAMETERS in various run configurations
README.md Updated Java bindings for FUSE to use Foreign Function & Memory API, now requiring JDK 22
jfuse-api/..., jfuse-linux-aarch64/..., jfuse-linux-amd64/... Modified FUSE operation handling, memory management, and method calls to align with JDK 22's requirements

"In the realm of code, where Java reigns supreme,
A leap to twenty-two, a developer's dream.
🐇 With every line refined, and APIs aligned,
Our rabbit hops ahead, leaving old bugs behind.
Through memory and FUSE, it dances with grace,
A leap, a bound, in the open-source space."

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[ci skip]
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 28

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f8c6839 and 54c6907.
Files ignored due to path filters (5)
  • jfuse-linux-aarch64/pom.xml is excluded by: !**/*.xml
  • jfuse-linux-amd64/pom.xml is excluded by: !**/*.xml
  • jfuse-mac/pom.xml is excluded by: !**/*.xml
  • jfuse-tests/pom.xml is excluded by: !**/*.xml
  • jfuse-win/pom.xml is excluded by: !**/*.xml
Files selected for processing (65)
  • .github/workflows/build.yml (4 hunks)
  • .github/workflows/publish-central.yml (1 hunks)
  • .github/workflows/publish-github.yml (1 hunks)
  • .idea/misc.xml (1 hunks)
  • .idea/runConfigurations/HelloWorldFileSystem.xml (1 hunks)
  • .idea/runConfigurations/HelloWorldFileSystem__Windows_.xml (1 hunks)
  • .idea/runConfigurations/PosixMirrorFileSystem__Linux_.xml (1 hunks)
  • .idea/runConfigurations/PosixMirrorFileSystem__fuse_t_.xml (1 hunks)
  • .idea/runConfigurations/PosixMirrorFileSystem__macFUSE_.xml (1 hunks)
  • .idea/runConfigurations/RandomFileSystem.xml (1 hunks)
  • .idea/runConfigurations/WindowsMirrorFileSystem.xml (1 hunks)
  • README.md (2 hunks)
  • jfuse-api/src/main/java/org/cryptomator/jfuse/api/util/MemoryUtils.java (1 hunks)
  • jfuse-api/src/test/java/org/cryptomator/jfuse/api/util/MemoryUtilsTest.java (2 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/DirFillerImpl.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FileInfoImpl.java (3 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseArgs.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseConfigImpl.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseConnInfoImpl.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseImpl.java (3 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseMountImpl.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/StatImpl.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/StatvfsImpl.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/TimeSpecImpl.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/errno/errno_h.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fcntl/fcntl_h.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_args.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_config.java (2 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_conn_info.java (2 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_file_info.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_fill_dir_t.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_h.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_loop_config_v1.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/stat.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/statvfs.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/timespec.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3_lowlevel/fuse_cmdline_opts.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3_lowlevel/fuse_lowlevel_h.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/stat/stat_h.java (1 hunks)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/stdio/stdio_h.java (1 hunks)
  • jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/FileInfoImplTest.java (1 hunks)
  • jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/FuseConfigImplTest.java (2 hunks)
  • jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/FuseConnInfoImplTest.java (4 hunks)
  • jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/FuseImplTest.java (12 hunks)
  • jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/StatImplTest.java (4 hunks)
  • jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/TimeSpecImplTest.java (4 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/DirFillerImpl.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FileInfoImpl.java (3 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseArgs.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseConfigImpl.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseConnInfoImpl.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseImpl.java (3 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseMountImpl.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/StatImpl.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/StatvfsImpl.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/TimeSpecImpl.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/errno/errno_h.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fcntl/fcntl_h.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_args.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_config.java (2 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_conn_info.java (2 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_file_info.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_fill_dir_t.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_h.java (1 hunks)
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_loop_config_v1.java (1 hunks)
Files not summarized due to errors (2)
  • jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_config.java: Error: Message exceeds token limit
  • jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_config.java: Error: Message exceeds token limit
Additional comments: 164
jfuse-api/src/main/java/org/cryptomator/jfuse/api/util/MemoryUtils.java (1)
  • 11-11: The change from getUtf8String to getString in toUtf8StringOrNull method aligns with the updates in JDK 22 and the new FFM API. It's a good practice to add a comment explaining such changes for future reference and clarity.
.idea/misc.xml (1)
  • 11-11: Updating the Java SDK version from 21 to 22 in the project configuration is consistent with the PR objectives. Ensure this change is reflected across all project configurations and documentation for consistency.
.idea/runConfigurations/HelloWorldFileSystem__Windows_.xml (1)
  • 6-6: The removal of the --enable-preview flag from VM_PARAMETERS is appropriate with the transition to JDK 22, assuming all previously previewed features are now stable. Verify that no preview features are still in use that would require this flag.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/DirFillerImpl.java (1)
  • 12-18: The changes in DirFillerImpl, including the update to the callback parameter type and the use of fuse_fill_dir_t.invoke, align with the transition to the new FFM API. Consider adding comments explaining these changes, especially if there are implications for memory management or performance.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/StatImpl.java (2)
  • 14-29: The updates to aTime, cTime, mTime, and birthTime methods in StatImpl class simplify the access to time fields by directly using the stat class methods without slicing. This change improves readability and potentially enhances performance by avoiding unnecessary operations.
  • 34-79: The modifications to setMode, getMode, setUid, getUid, setGid, getGid, setNLink, getNLink, setSize, and getSize methods in StatImpl class for setting and getting file attributes directly access the memory segment fields through the stat class. These changes are consistent with the objective of simplifying method calls and align with best practices for working with the Foreign Function & Memory API.
.github/workflows/publish-github.yml (1)
  • 13-13: Updating the Java version to 22-ea in the GitHub Actions workflow publish-github.yml aligns with the PR's objective to transition the project to JDK 22. This change ensures that the CI/CD pipeline uses the correct Java version for building and publishing.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseMountImpl.java (1)
  • 25-26: The changes in FuseMountImpl class to use fuse_loop_config_v1.clone_fd and fuse_loop_config_v1.max_idle_threads for setting FUSE loop configuration parameters directly are in line with the updated FFM API practices. These modifications simplify the code and potentially improve performance by using direct method calls.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseMountImpl.java (1)
  • 25-26: The updates in the FuseMountImpl class for the aarch64 package to use fuse_loop_config_v1.clone_fd and fuse_loop_config_v1.max_idle_threads directly for setting FUSE loop configuration parameters align with the project's transition to JDK 22 and the new FFM API. These changes simplify the code and are consistent with best practices.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseConnInfoImpl.java (1)
  • 13-93: The refactoring in FuseConnInfoImpl class to use direct method calls for accessing and setting FUSE connection information fields simplifies the code and aligns with the objectives of enhancing maintainability and performance. This approach is consistent with best practices for working with the Foreign Function & Memory API.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseConnInfoImpl.java (1)
  • 13-93: The changes in the FuseConnInfoImpl class for the aarch64 package to directly access and set FUSE connection information fields through method calls are appropriate and improve code clarity. These modifications are in line with the project's goal of leveraging the new FFM API more effectively.
jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/TimeSpecImplTest.java (1)
  • 19-20: The updates in TimeSpecImplTest to use timespec.tv_sec and timespec.tv_nsec directly for setting time values in test cases align with the simplifications made in the main codebase. These changes ensure that the tests remain relevant and accurate in testing the functionality of the TimeSpecImpl class after the transition to the new FFM API.

Also applies to: 34-35, 49-50, 62-63

jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseConfigImpl.java (1)
  • 13-118: The modifications in the FuseConfigImpl class to use direct method calls for accessing and setting FUSE configuration values streamline the code and enhance readability. These changes are consistent with the project's objectives to simplify interactions with the FFM API and improve maintainability.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseConfigImpl.java (1)
  • 13-118: The changes in the FuseConfigImpl class for the aarch64 package to directly access and set FUSE configuration values through method calls are well-implemented. These updates simplify the code and align with the project's goal of leveraging the new FFM API more effectively.
jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/FileInfoImplTest.java (1)
  • 25-25: The update in FileInfoImplTest to use fuse_file_info.flags directly for setting flags in test cases is consistent with the simplifications made in the main codebase. This change ensures that the test remains relevant and accurately reflects the functionality of the FileInfoImpl class after the API updates.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FileInfoImpl.java (2)
  • 26-33: The introduction of a null-safe factory method ofNullable in FileInfoImpl class is a good practice, ensuring that the code gracefully handles null MemorySegment instances. This change enhances the robustness of the code.
  • 23-51: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [38-88]

The updates to access and set fields in fuse_file_info directly in the FileInfoImpl class, including getFh, setFh, getFlags, and getLockOwner, simplify the code and improve its readability. These changes are consistent with the project's objectives to leverage the new FFM API more effectively.

jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FileInfoImpl.java (2)
  • 26-33: The addition of a null-safe factory method ofNullable in the FileInfoImpl class for the aarch64 package is a positive change, enhancing code safety and maintainability by gracefully handling potential null pointers.
  • 23-51: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [38-88]

The modifications in the FileInfoImpl class to directly access and set fields in fuse_file_info, including getFh, setFh, getFlags, and getLockOwner, simplify interactions with the FFM API. These changes align with the project's goal of improving code maintainability and performance.

jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_fill_dir_t.java (5)
  • 38-41: The constructor of fuse_fill_dir_t is made package-private, which is a good practice for utility classes that are not intended to be instantiated externally. However, consider adding a comment inside the constructor body to explain why direct instantiation is discouraged.
  • 47-48: The nested functional interface Function is correctly defined to match the expected function pointer signature. This is a good use of Java's functional interfaces to represent C function pointers in a type-safe manner.
  • 51-58: The FunctionDescriptor is correctly defined with the appropriate types for the fuse_fill_dir_t function pointer. This ensures type safety and clarity when interfacing with native code.
  • 73-74: The allocate method correctly binds the functional interface to an upcall stub, allowing Java code to be called from native code. However, ensure that the Arena parameter's usage aligns with the updated memory management model in JDK 22, as direct Arena usage is deprecated in favor of more explicit memory management techniques.
  • 82-87: The invoke method correctly handles the invocation of the upcall stub with the provided parameters. The use of invokeExact and the explicit cast to int ensure type safety and match the expected return type. The error handling with AssertionError is appropriate for unexpected conditions in this context.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_fill_dir_t.java (5)
  • 39-42: The constructor of fuse_fill_dir_t for the jfuse-linux-aarch64 module is correctly defined as package-private, similar to the amd64 version. This consistency across modules is good for maintainability. Including a comment on why instantiation is discouraged would still be beneficial.
  • 48-49: The nested functional interface Function is consistently defined across both amd64 and aarch64 modules, ensuring a uniform approach to representing C function pointers in Java. This consistency is crucial for maintainability and understanding across different platform modules.
  • 52-59: The FunctionDescriptor is correctly defined for the aarch64 architecture, matching the expected function pointer signature. This ensures that the interface with native code is type-safe and clear, which is essential for cross-platform compatibility.
  • 74-75: The allocate method's implementation is consistent with the amd64 version, correctly binding the functional interface to an upcall stub. The same verification regarding the Arena parameter's usage applies here, considering potential deprecation in JDK 22.
  • 83-87: The invoke method's implementation is correct and consistent with the amd64 version, using invokeExact for type safety and matching the expected return type. The error handling approach is appropriate for this context.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3_lowlevel/fuse_lowlevel_h.java (6)
  • 21-22: The introduction of the LIBRARY_ARENA field aligns with the changes in memory management in JDK 22. However, as previously mentioned, verify that the use of Arena is consistent with the latest best practices and recommendations in JDK 22, especially considering its potential deprecation.
  • 24-29: The traceDowncall method is a useful addition for debugging and tracing native calls. This method enhances maintainability and debugging capabilities by providing a standardized way to log downcall information. Ensure that this tracing mechanism is used consistently across the codebase where downcalls are made.
  • 31-33: The findOrThrow method correctly encapsulates the logic for symbol lookup and error handling. This method improves code readability and error handling by centralizing the symbol lookup process and throwing a descriptive error when a symbol is unresolved.
  • 36-41: The upcallHandle method correctly retrieves a method handle for a given function interface and descriptor. This method is crucial for setting up upcalls, ensuring that Java methods can be called from native code in a type-safe manner. The error handling with AssertionError is appropriate for unexpected conditions.
  • 44-55: The align method provides a flexible way to adjust the byte alignment of memory layouts. This method is particularly useful for ensuring compatibility with native code expectations and can improve performance by aligning data structures according to the native platform's requirements.
  • 61-70: The update to data type declarations using ValueLayout instead of previous types (e.g., OfByte, OfShort, etc.) aligns with the changes in JDK 22 and enhances type safety and clarity. This update is a positive change, ensuring that the codebase is using the most up-to-date and appropriate data type representations.
jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/FuseConnInfoImplTest.java (4)
  • 26-26: The removal of the arena parameter from the FuseConnInfoImpl constructor call aligns with the updated memory management model in JDK 22. This change simplifies object instantiation and is consistent with the PR objectives.
  • 36-45: The update in method references within the testGetters method streamlines the test definitions by removing redundant setter method suffixes. This change enhances readability and maintainability of the test code.
  • 59-59: Similar to the testGetters method, the removal of the arena parameter in the FuseConnInfoImpl constructor call within the testSetters method is consistent with the PR objectives and improves code simplicity.
  • 69-75: The updates in the testSetters method, like in the testGetters method, improve the clarity and maintainability of the test code by focusing on the essential aspects of the tested functionality.
jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/FuseConfigImplTest.java (2)
  • 26-26: The removal of the arena parameter from the FuseConfigImpl constructor call is consistent with the updated memory management model in JDK 22. This simplification aligns with the PR objectives and enhances code readability.
  • 36-57: The updates in method signatures within the testGetters method reflect the changes in the fuse_config method calls. These adjustments ensure that the tests accurately represent the updated API and functionalities, contributing to the overall maintainability and correctness of the test suite.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fcntl/fcntl_h.java (5)
  • 21-29: The introduction of a method to trace downcalls (traceDowncall) provides a useful debugging tool that can help in identifying issues with native calls. This addition is thoughtful and can significantly aid in development and troubleshooting.
  • 31-34: The findOrThrow method for symbol lookup is a robust way to ensure that symbols are correctly resolved or provide a clear error message otherwise. This approach enhances error handling and debugging capabilities.
  • 36-42: The method upcallHandle simplifies the creation of method handles for upcalls, encapsulating the reflective operations and error handling in a concise manner. This utility method contributes to code cleanliness and maintainability.
  • 44-56: The align method for memory layout alignment introduces flexibility in handling memory layouts, allowing for custom alignments. This utility is particularly useful in dealing with native code where alignment can be critical for correct operation.
  • 61-70: Refactoring constants for file control options to use private static final integers instead of direct values improves code readability and maintainability. This change also aligns with best practices for defining constants in Java.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fcntl/fcntl_h.java (5)
  • 21-29: The addition of a method to trace downcalls (traceDowncall) in the aarch64 version, similar to the amd64 version, is a valuable debugging tool that enhances the development and troubleshooting process.
  • 31-34: The findOrThrow method for symbol lookup ensures robust error handling by providing clear error messages when symbols cannot be resolved. This utility method is crucial for debugging and maintaining code quality.
  • 36-42: The upcallHandle method simplifies the creation of method handles for upcalls, encapsulating reflective operations and error handling. This utility method is a valuable addition to the codebase, enhancing code cleanliness and maintainability.
  • 44-56: The align method for memory layout alignment provides a flexible solution for custom alignments in memory layouts, which is essential when dealing with native code. This utility method is a thoughtful addition to the toolkit for handling native interactions.
  • 61-70: The introduction of constants for file control options using private static final integers instead of direct values in the aarch64 version, as in the amd64 version, improves code readability and aligns with best practices for defining constants in Java.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/timespec.java (3)
  • 29-33: The redefinition of the timespec class layout to include tv_sec and tv_nsec fields aligns with the struct definition in C. This change ensures that the Java representation matches the native struct, facilitating correct memory manipulation and data access.
  • 71-82: The addition of getter and setter methods for the tv_sec and tv_nsec fields provides a clear and safe way to access and modify these fields within the timespec struct. This approach enhances code readability and safety by encapsulating direct memory access.
  • 133-170: The introduction of methods for allocating memory segments, creating slices, and reinterpreting memory addresses for the timespec struct adds flexibility and utility for working with instances of this struct in memory. These methods are crucial for efficient and safe memory management in the context of interacting with native code.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_loop_config_v1.java (3)
  • 25-26: The constructor is private, ensuring that instances of fuse_loop_config_v1 cannot be directly instantiated, which is appropriate for a utility class that primarily provides static methods and constants.
  • 29-32: The struct layout is correctly defined with the appropriate field names and types. It's good practice to explicitly name the struct to match its C counterpart for clarity.
  • 129-135: The asSlice method provides a way to obtain a slice of a memory segment representing an array of fuse_loop_config_v1 structs. This utility method is correctly implemented and can be useful for working with arrays of this struct in memory.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/errno/errno_h.java (3)
  • 31-34: The findOrThrow method is a robust way to handle unresolved symbols by throwing an UnsatisfiedLinkError. This approach enhances error handling by providing clear feedback when a symbol cannot be found.
  • 44-56: The align method provides a flexible way to adjust the byte alignment of various memory layouts. This method is particularly useful when dealing with native code that has specific alignment requirements. The implementation appears correct and efficiently handles different types of MemoryLayout.
  • 58-69: The use of ValueLayout constants (C_BOOL, C_CHAR, etc.) is a significant improvement over previous versions that might have used less type-safe approaches. This change enhances type safety and readability.
README.md (1)
  • 13-14: Updating the JDK version requirement in the README.md to JDK 22 is crucial for ensuring that users are aware of the new prerequisites for the project. This change is clearly communicated and aligns with the project's transition to the new Foreign Function & Memory API.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_file_info.java (2)
  • 41-48: The layout definition for fuse_file_info struct is correctly updated to reflect the specific fields and their sizes. This change is crucial for ensuring that the Java representation matches the native struct layout accurately. The use of MemoryLayout.structLayout and withName for each field enhances readability and maintainability.
  • 57-98: The getter and setter methods for the flags field (and similarly for other fields like fh, lock_owner, and poll_events) are correctly implemented. They use the layout and offset information to access the struct fields properly. This approach ensures type safety and alignment with the native memory layout.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_file_info.java (2)
  • 41-48: The layout definition for fuse_file_info struct in the aarch64 package mirrors the changes made in the amd64 package, ensuring consistency across architectures. This uniform approach is essential for maintaining the integrity of the project across different platforms.
  • 57-98: The implementation of getter and setter methods for struct fields such as flags, fh, lock_owner, and poll_events is consistent with the amd64 version. This consistency is key to ensuring that the project's codebase remains maintainable and understandable across different platform-specific modules.
jfuse-linux-aarch64/src/test/java/org/cryptomator/jfuse/linux/aarch64/FuseImplTest.java (12)
  • 89-89: The direct method call to fuse_cmdline_opts.show_help(opts, 1) is a good update, aligning with the new API standards. However, ensure that the fuse_cmdline_opts class's methods are correctly updated to support this direct invocation pattern.
  • 106-108: The use of direct method calls for fuse_cmdline_opts.singlethread(opts, 0), fuse_cmdline_opts.debug(opts, 1), and fuse_cmdline_opts.mountpoint(opts, arena.allocateFrom("/mount/point")) is a positive change, simplifying the code and making it more readable. However, it's crucial to verify that these methods properly handle the memory segments, especially with the allocation for the mount point, to avoid memory leaks or incorrect memory management.
  • 132-132: Allocating memory for the path using arena.allocateFrom("/foo") in the testFlush method is consistent with the new memory management model. Ensure that all memory allocated within the test is properly closed or managed to prevent memory leaks.
  • 146-146: Similar to the previous comment, the allocation for the path in testFsync method follows the updated memory management practices. It's important to maintain this consistency across all tests and ensure proper memory management.
  • 160-160: The allocation for the path in testFsyncdir method is correctly updated. As with other test methods, verify that memory management is handled correctly to avoid leaks.
  • 199-199: The allocation for the path in testUtimensNow method without specifying timespec values indicates a use case for setting both access and modification times to the current time. This test should ensure that the utimens operation correctly interprets MemorySegment.NULL as an instruction to use the current time.
  • 220-226: In testUtimens, the detailed allocation and setup of timespec values for access and modification times demonstrate a thorough test case for the utimens operation. Ensure that the timespec utility methods are correctly used and that the memory is managed properly to reflect real-world usage accurately.
  • 244-245: The allocation for the path and name in testGetxattr method follows the updated memory management model. It's crucial to ensure that the allocated memory is managed correctly throughout the test to prevent potential memory leaks.
  • 260-261: The memory allocation for path and name in the testSetxattr method is consistent with the new API standards. As with other tests, proper memory management is essential to avoid leaks.
  • 276-276: The allocation for the path in testListxattr method is correctly implemented. Ensure that the test accurately reflects the expected behavior and that memory management practices are followed.
  • 291-292: In testRemovexattr, the memory allocation for path and name is consistent with the updated API. It's important to verify that these allocations are managed correctly to prevent memory issues.
  • 308-308: The allocation for the path in testChown method is correctly updated to align with the new memory management model. Ensure that the test covers the expected behavior and that memory is managed properly.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/FuseImpl.java (32)
  • 45-53: The updated method for parsing command line arguments, including the allocation of argv and setting its elements, is a significant improvement. It's important to ensure that the memory allocated for argv and the strings within it are managed correctly to avoid memory leaks.
  • 60-60: The handling of the show_help option within parseArgs is correctly implemented. However, ensure that the fuse_h.fuse_lib_help(args) method is called with the correct arguments and that any allocated memory is properly managed.
  • 70-99: The method bind efficiently allocates and sets up the operation handlers for the FUSE operations. It's crucial to verify that each operation's allocation within the fuseArena is managed correctly to prevent memory leaks or incorrect behavior.
  • 105-108: The init method's implementation, including setting the FUSE_CAP_READDIRPLUS flag, is correctly updated. Ensure that the memory management for conn and cfg segments is handled properly.
  • 113-113: The access method correctly retrieves the path string for the operation. It's important to ensure that the string retrieval from the memory segment is done correctly and efficiently.
  • 117-117: The chmod method's implementation is correct, ensuring proper string retrieval and handling of the FileInfoImpl. Verify that the memory management for the path segment is handled correctly.
  • 122-122: The chown method correctly retrieves the path string and handles the FileInfoImpl. Ensure that the memory management for the path segment and the FileInfoImpl is correctly implemented.
  • 126-126: The create method's implementation, including string retrieval and FileInfoImpl handling, is correct. Verify that the memory management for the path segment is handled properly.
  • 135-135: The flush method correctly retrieves the path string and handles the FileInfoImpl. Ensure that the memory management for the path segment and the FileInfoImpl is correctly implemented.
  • 140-140: The fsync method's implementation is correct, ensuring proper string retrieval and handling of the FileInfoImpl. Verify that the memory management for the path segment is handled correctly.
  • 145-145: The fsyncdir method correctly converts the path to a UTF-8 string and handles the FileInfoImpl. It's important to ensure that the memory management for the path segment and the FileInfoImpl is correctly implemented.
  • 149-149: The getattr method correctly retrieves the path string and handles both StatImpl and FileInfoImpl. Ensure that the memory management for the path segment is handled properly.
  • 155-155: The getxattr method correctly retrieves the path and name strings and handles the value buffer. Verify that the memory management for the path, name segments, and the value buffer is correctly implemented.
  • 161-161: The setxattr method correctly retrieves the path and name strings and handles the value buffer. Ensure that the memory management for the path, name segments, and the value buffer is correctly implemented.
  • 167-167: The listxattr method correctly retrieves the path string and handles the value buffer. Verify that the memory management for the path segment and the value buffer is correctly implemented.
  • 172-172: The removexattr method correctly retrieves the path and name strings. Ensure that the memory management for the path and name segments is correctly implemented.
  • 176-176: The mkdir method correctly retrieves the path string for the operation. It's important to ensure that the string retrieval from the memory segment is done correctly and efficiently.
  • 180-180: The open method correctly retrieves the path string and handles the FileInfoImpl. Ensure that the memory management for the path segment and the FileInfoImpl is correctly implemented.
  • 184-184: The opendir method correctly retrieves the path string and handles the FileInfoImpl. Ensure that the memory management for the path segment and the FileInfoImpl is correctly implemented.
  • 188-189: The read method correctly retrieves the path string, handles the buffer, and manages the FileInfoImpl. Verify that the memory management for the path segment, buffer, and the FileInfoImpl is correctly implemented.
  • 194-194: The readdir method correctly retrieves the path string and handles the DirFillerImpl and FileInfoImpl. Ensure that the memory management for the path segment and the DirFillerImpl is correctly implemented.
  • 200-200: The readlink method correctly retrieves the path string and handles the buffer. Verify that the memory management for the path segment and the buffer is correctly implemented.
  • 204-204: The release method correctly retrieves the path string and handles the FileInfoImpl. Ensure that the memory management for the path segment and the FileInfoImpl is correctly implemented.
  • 208-208: The releasedir method correctly converts the path to a UTF-8 string and handles the FileInfoImpl. It's important to ensure that the memory management for the path segment and the FileInfoImpl is correctly implemented.
  • 212-212: The rename method correctly retrieves the old and new path strings. Ensure that the memory management for both path segments is correctly implemented.
  • 216-216: The rmdir method correctly retrieves the path string for the operation. It's important to ensure that the string retrieval from the memory segment is done correctly and efficiently.
  • 220-220: The statfs method correctly retrieves the path string and handles the StatvfsImpl. Verify that the memory management for the path segment is handled properly.
  • 224-224: The symlink method correctly retrieves the link name and target strings. Ensure that the memory management for both string segments is correctly implemented.
  • 228-228: The truncate method correctly retrieves the path string and handles the FileInfoImpl. Ensure that the memory management for the path segment and the FileInfoImpl is correctly implemented.
  • 232-232: The unlink method correctly retrieves the path string for the operation. It's important to ensure that the string retrieval from the memory segment is done correctly and efficiently.
  • 240-248: The utimens method's handling of times, especially the conditional logic for setting current time or specific times, is correctly implemented. Ensure that the memory management for the times segment and the allocated timespec segments is correctly handled to prevent memory leaks.
  • 254-255: The write method correctly retrieves the path string, handles the buffer, and manages the FileInfoImpl. Verify that the memory management for the path segment, buffer, and the FileInfoImpl is correctly implemented.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/FuseImpl.java (4)
  • 70-99: The method bind uses a switch expression to allocate memory for various file system operations. Each case correctly allocates memory for the specific operation using fuseArena. However, it's important to ensure that fuseArena is properly managed to avoid memory leaks, especially since these operations are likely to be called frequently. Additionally, the use of fuseArena across multiple operations raises questions about its lifecycle and management.

Consider verifying the lifecycle management of fuseArena to prevent memory leaks and ensure efficient memory usage.

  • 113-113: The access method uses path.getString(0) to retrieve the path string from the memory segment. This approach assumes that the memory segment represents a null-terminated string starting at offset 0. It's crucial to ensure that this assumption holds true for all calls to this method to prevent unexpected behavior or errors.

Verify that all memory segments passed to the access method represent null-terminated strings starting at offset 0.

  • 145-145: In the fsyncdir method, the usage of MemoryUtils.toUtf8StringOrNull(path) is a good practice for safely converting memory segments to strings, potentially handling NULL segments gracefully. However, ensure that MemoryUtils.toUtf8StringOrNull is robust against various edge cases, such as empty strings or invalid UTF-8 sequences.

Review the implementation of MemoryUtils.toUtf8StringOrNull to ensure it handles edge cases appropriately.

  • 242-250: The utimens method demonstrates careful handling of the times memory segment, allocating and setting timespec structures as needed. This method's implementation is a good example of managing memory segments and conditional logic based on the NULL check. However, ensure that the allocated timespec segments are properly released or managed to avoid memory leaks.

Confirm that memory allocated within the utimens method is properly managed and released to prevent memory leaks.

jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_conn_info.java (5)
  • 5-14: The imports and static imports are correctly updated to reflect the usage of the new Foreign Function & Memory API (FFM API). This ensures that the file has access to the necessary classes and methods for memory management and struct layout definitions.
  • 2-19: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [16-29]

The documentation provided through the JavaDoc comment is clear and helpful. It includes a C snippet that describes the structure of the fuse_conn_info struct, which aids in understanding the Java representation of this struct. Including such snippets is a good practice for maintainability and clarity, especially when dealing with native code interop.

  • 34-35: The constructor of fuse_conn_info is private, which is appropriate since this class is meant to represent a native struct and should not be instantiated directly. This follows the principle of encapsulation and ensures that instances of fuse_conn_info are only created through the appropriate memory management mechanisms.
  • 38-50: The layout definition for fuse_conn_info using MemoryLayout.structLayout is correctly implemented. It accurately reflects the structure of the native fuse_conn_info struct, including the sequence layout for the reserved array. This is crucial for correct memory access and manipulation of the struct fields.
  • 52-587: The methods for layout access, offset calculation, getters, and setters for each field are correctly implemented. These methods provide a safe and structured way to access and modify the fields of the fuse_conn_info struct in memory. The use of MemorySegment and VarHandle is consistent with the FFM API's approach to memory management and access, ensuring type safety and performance.

However, it's important to ensure that the offsets and layouts used in these methods are thoroughly tested, especially considering the manual calculations and selections involved. Incorrect offsets or layouts can lead to subtle and hard-to-diagnose bugs in memory access.

jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_conn_info.java (5)
  • 5-14: The imports and static imports are correctly updated to use the new FFM API classes and methods. This aligns with the JDK 22 update and the transition to the new API.
  • 2-19: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [16-29]

The class documentation provides a clear C struct representation which is helpful for understanding the mapping between the C struct and the Java class. Including such documentation enhances maintainability and readability.

  • 34-35: The constructor of fuse_conn_info is private, which is appropriate since instances of this class are likely managed through static methods and direct memory allocation rather than through traditional instantiation. This approach is consistent with the usage patterns expected in low-level system programming interfaces.
  • 38-50: The layout definition using MemoryLayout.structLayout is correctly defined, matching the C struct layout. This ensures that the Java representation will correctly interact with native code. The use of named fields within the layout enhances readability and maintainability.
  • 589-602: The allocation methods allocate and allocateArray are correctly implemented using the SegmentAllocator interface, which is part of the new memory management model in JDK 22. This approach is more flexible and safer compared to manual memory management.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/statvfs.java (8)
  • 5-14: The import statements and static imports are correctly updated to use the new FFM API classes and methods. This ensures compatibility with JDK 22 and leverages the latest API improvements for memory management and foreign function access.
  • 39-52: The layout definition for the statvfs struct is correctly updated to reflect the new memory layout as per the FFM API. Each field is appropriately named and aligned with the struct definition in C, ensuring accurate memory mapping for native interoperability.
  • 91-102: The getter and setter methods for f_bsize are correctly implemented, providing safe and efficient access to the struct field. The use of layout and offset ensures type safety and performance. These methods follow the best practices for memory segment operations in the FFM API.
  • 105-126: Similar to the f_bsize field, the layout, offset, getter, and setter methods for f_frsize are correctly implemented. This pattern is consistently applied across all fields, ensuring uniformity and adherence to best practices in memory segment operations.
  • 149-170: The layout, offset, getter, and setter methods for f_blocks are correctly implemented. This consistent approach across all fields ensures that the class provides a robust and type-safe interface for accessing and modifying the statvfs struct fields.
  • 585-587: The setter method for the __f_spare array field correctly uses MemorySegment.copy for copying the array content. This method ensures efficient and safe copying of array elements between memory segments. It's a good use of the FFM API for handling array fields within structs.
  • 626-628: The asSlice utility method provides a convenient way to obtain a slice of an array field within the struct. This method enhances the class's usability by allowing easy access to individual array elements. It's a useful addition to the class's API.
  • 638-647: The allocation methods (allocate and allocateArray) are correctly implemented using the SegmentAllocator interface. These methods provide a convenient and safe way to allocate memory for the statvfs struct and arrays of the struct, aligning with the best practices for memory management in the FFM API.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/stat.java (2)
  • 43-59: The struct layout definition is comprehensive and aligns well with the C struct it represents. However, it's important to ensure that the data types used (C_LONG, C_INT, etc.) accurately reflect the corresponding C types for each field across different platforms. This is crucial for maintaining compatibility and preventing data loss or corruption.
  • 597-619: The layout for struct timespec fields (st_atim, st_mtim, st_ctim) is correctly defined using a GroupLayout. However, ensure that the timespec layout itself is correctly defined elsewhere and accurately represents the corresponding C struct. This is crucial for correct interpretation of time-related fields.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_h.java (20)
  • 21-22: The use of a static Arena and the TRACE_DOWNCALLS flag for debugging purposes is a good practice for managing native memory and tracing native calls. However, ensure that the Arena usage aligns with the updated memory management model in JDK 22 and that there are no memory leaks.
  • 31-34: The findOrThrow method is crucial for ensuring that symbols are correctly resolved. It's well-implemented with clear error handling. However, verify that all symbols used in this class are available in the target environments to prevent UnsatisfiedLinkError.
  • 36-42: The upcallHandle method correctly retrieves method handles for virtual methods. Ensure that the method names and descriptors passed to this method accurately reflect the methods intended for upcalls, to avoid ReflectiveOperationException.
  • 44-55: The align method provides a flexible way to adjust memory layouts. This is particularly important for ensuring compatibility with different architectures and alignment requirements. Ensure that this method is used consistently across all relevant memory layouts in the project.
  • 58-59: The SYMBOL_LOOKUP initialization using SymbolLookup.loaderLookup() combined with Linker.nativeLinker().defaultLookup() is a robust approach for symbol resolution. This ensures compatibility with both the module system and native libraries.
  • 61-70: The definition of common C types using ValueLayout and AddressLayout is correctly aligned with the FFM API. This standardization facilitates the handling of native types and ensures type safety across FUSE operations.
  • 72-78: The implementation of the fuse_version function, including its descriptor, handle, and the actual method, is a good example of how to wrap native functions using the FFM API. Ensure that all such functions are tested thoroughly to validate their correctness and handle any potential exceptions gracefully.

Also applies to: 87-99, 106-113

  • 117-123: Similar to the fuse_version function, the fuse_loop_cfg_create function is well-implemented. Pay special attention to memory management, especially the returned MemorySegment, to ensure it is properly closed or managed to avoid memory leaks.

Also applies to: 132-144, 151-158

  • 162-168: The fuse_loop_cfg_destroy function demonstrates proper handling of resource cleanup. It's important to ensure that all resources allocated through the FFM API are correctly released to prevent memory leaks.

Also applies to: 178-189, 197-204

  • 208-216: The method fuse_loop_cfg_set_max_threads correctly demonstrates how to pass parameters to native functions. Ensure that the types and values of all parameters passed to native functions are validated to prevent undefined behavior.

Also applies to: 225-236, 244-251

  • 255-263: The fuse_loop_cfg_set_clone_fd function is another example of interacting with native code. As with other functions, ensure that the parameters and return types are correctly handled according to the native function's expectations.

Also applies to: 272-283, 291-298

  • 302-309: The fuse_lib_help function, which likely provides help or documentation for the FUSE library, is correctly implemented. It's a good practice to include such helper functions, especially when dealing with complex native libraries.

Also applies to: 318-329, 337-344

  • 348-359: The fuse_new function, responsible for creating a new FUSE instance, is critical for the library's functionality. Ensure that the memory allocated for the new instance is managed correctly and that any errors during creation are handled appropriately.

Also applies to: 368-379, 387-394

  • 398-407: The fuse_mount function is essential for mounting the filesystem. It's important to ensure that error handling is robust, especially for operations that interact with the filesystem, to gracefully handle failures.

Also applies to: 416-427, 435-442

  • 446-453: The fuse_unmount function is crucial for resource cleanup. Similar to fuse_destroy, ensure that this function is called appropriately to prevent resource leaks or dangling mounts.

Also applies to: 462-473, 481-488

  • 492-499: The fuse_destroy function is key for releasing resources associated with a FUSE instance. Proper error handling and resource management in this function are critical to prevent memory leaks.

Also applies to: 508-519, 527-534

  • 538-546: The fuse_loop function, which likely enters the main FUSE event loop, is a core part of the library's functionality. Ensure that this function is robust and handles any runtime exceptions or errors gracefully.

Also applies to: 555-566, 574-581

  • 585-592: The fuse_exit function provides a mechanism to exit the FUSE loop. It's important to ensure that this function can be called safely at any point and that it correctly cleans up resources.

Also applies to: 601-612, 620-627

  • 631-640: The fuse_loop_mt function introduces multi-threaded support for the FUSE loop. Given the complexity of multi-threaded operations, ensure that this function is thoroughly tested for thread safety and performance.

Also applies to: 649-660, 668-675

  • 679-687: The fuse_get_session function is important for session management. Ensure that the session objects are correctly managed and that any associated resources are released when no longer needed.

Also applies to: 696-707, 715-722

jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_h.java (4)
  • 5-11: The import statements are correctly updated to reflect the usage of the new Foreign Function & Memory API (FFM API) components such as java.lang.invoke, java.lang.foreign, and related utilities. This aligns with the transition to JDK 22 and the use of the FFM API.
  • 31-34: The findOrThrow method provides a clean and concise way to handle symbol lookup failures. Using orElseThrow with a custom error message enhances the readability and maintainability of the code. This is a good practice for error handling in symbol lookup operations.
  • 44-56: The align method is a clever utility for aligning memory layouts, which is crucial for interoperability with native code. The use of pattern matching with switch expressions enhances readability and maintainability. This method demonstrates a deep understanding of memory layout manipulation in the context of the FFM API.
  • 72-720: Overall, the refactoring of the fuse_h class to utilize the new FFM API is well-executed. The transition to using MemorySegment for memory management and the introduction of method handles and function descriptors for FUSE operations are in line with the objectives of updating the project to JDK 22. The code is generally clean, well-structured, and adheres to Java best practices. Minor improvements in error handling and documentation could further enhance the maintainability and usability of the code.
jfuse-linux-amd64/src/main/java/org/cryptomator/jfuse/linux/amd64/extr/fuse3/fuse_config.java (3)
  • 5-14: The imports are correctly updated to use the new java.lang.foreign package, aligning with the JDK 22 update and the transition to the FFM API. This change is consistent with the PR objectives.
  • 48-49: The constructor of fuse_config is private, which is appropriate since this class represents a struct and instances are likely managed through memory segments rather than direct instantiation. This design choice aligns well with the usage patterns expected with the FFM API.
  • 82-1208: The methods provided for each field, including layout accessors, offset accessors, getters, and setters, are correctly implemented and follow the conventions of the FFM API. This ensures safe and efficient access to the native memory representing the fuse_config struct. The utility methods at the end of the class (asSlice, sizeof, allocate, allocateArray, reinterpret) are well-implemented and provide essential functionality for working with memory segments and arrays of fuse_config.
jfuse-linux-aarch64/src/main/java/org/cryptomator/jfuse/linux/aarch64/extr/fuse3/fuse_config.java (4)
  • 52-80: The static layout $LAYOUT is well-defined, matching the structure described in the comment block. This layout is crucial for the correct mapping between Java and native memory. It's good to see the use of MemoryLayout.structLayout to define the structure, which aligns with the FFM API's way of handling native structures. No changes are needed here.
  • 82-86: The method layout() provides public access to the $LAYOUT field, which is a good practice for encapsulation and reuse of the layout definition. This allows other parts of the codebase to use the layout without directly accessing the private field.
  • 1203-1206: The method allocate(SegmentAllocator allocator) is a convenient way to allocate memory for a single instance of the struct using a given allocator. This aligns with the FFM API's approach to memory management, allowing for more controlled allocation and deallocation of native memory. No changes are needed here.
  • 1210-1214: Similarly, allocateArray(long elementCount, SegmentAllocator allocator) provides a way to allocate memory for an array of structs, which is useful for working with multiple instances of the struct in native code. This method correctly calculates the total size needed for the array and uses the allocator to allocate the memory. This is a good practice for managing memory for arrays of native structs.

@overheadhunter
Copy link
Member Author

@coderabbitai please ignore all classes inside of **.extr.** packages, since these are machine-generated.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 54c6907 and be371d0.
Files selected for processing (2)
  • README.md (2 hunks)
  • jfuse-examples/README.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • README.md
Additional comments: 2
jfuse-examples/README.md (2)
  • 5-5: The update to require JDK 22 or newer is correctly documented, aligning with the project's transition to JDK 22 and the new FFM API. This change ensures users are aware of the new JDK requirement for running the examples.
  • 2-8: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [7-23]

The example commands for running the jFUSE examples correctly specify the use of --enable-native-access with the appropriate modules. This is consistent with the JDK 22 update and the new FFM API usage. However, it's important to ensure that the modules listed (org.cryptomator.jfuse.mac, org.cryptomator.jfuse.linux.arm64, org.cryptomator.jfuse.linux.amd64, and org.cryptomator.jfuse.win.amd64) are correctly updated to reflect any changes in module names or structure as a result of the project's transition to JDK 22.

This comment was marked as off-topic.

@overheadhunter

This comment was marked as off-topic.

This comment was marked as off-topic.

@overheadhunter

This comment was marked as off-topic.

This comment was marked as off-topic.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: .coderabbit.yaml

Commits Files that changed from the base of the PR and between be371d0 and fa90ca8.
Files selected for processing (3)
  • .github/workflows/build.yml (4 hunks)
  • .github/workflows/publish-central.yml (1 hunks)
  • .github/workflows/publish-github.yml (1 hunks)
Additional comments: 6
.github/workflows/publish-central.yml (1)
  • 18-18: The update of the Java version to 22 aligns with the PR objectives to leverage the new FFM API. This change is correctly implemented in the workflow configuration.
.github/workflows/publish-github.yml (1)
  • 13-13: The update of the Java version to 22 is correctly implemented in the workflow configuration for publishing to GitHub Packages. This aligns with the PR objectives.
.github/workflows/build.yml (4)
  • 15-15: The Java version update to 22 in the linux-amd64 job is correctly implemented. This is consistent with the PR objectives.
  • 38-38: The Java version update to 22 in the mac job is correctly implemented. This ensures compatibility with the latest Java standards.
  • 62-62: The Java version update to 22 in the win job is correctly implemented. This aligns with the PR's goal of leveraging the new FFM API.
  • 86-86: The Java version update to 22 in the sonarcloud job is correctly implemented, enhancing maintainability and performance.

Comment on lines 15 to 21
ref: "refs/tags/${{ github.event.inputs.tag }}"
- uses: actions/setup-java@v4
with:
java-version: 21
java-version: 22
distribution: 'zulu'
cache: 'maven'
server-id: ossrh # Value of the distributionManagement/repository/id field of the pom.xml
Copy link

@coderabbitai coderabbitai bot Mar 21, 2024

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [27-27]

Using variable interpolation ${{...}} with github context data in a run: step could potentially introduce security risks. Consider using an intermediate environment variable to store the data and use it in the run: script.

- run: mvn versions:set -B -DnewVersion=${{ github.event.inputs.tag }}
+ env:
+   TAG_VERSION: ${{ github.event.inputs.tag }}
+ run: mvn versions:set -B -DnewVersion="$TAG_VERSION"

Copy link
Member Author

Choose a reason for hiding this comment

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

Good hint, but out of scope of this PR

Copy link

Choose a reason for hiding this comment

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

@overheadhunter, understood and respect your decision on the scope of this PR. The security aspect of variable interpolation is indeed important, but addressing it at the right time is equally crucial. Would you like me to open a GitHub issue to track this for future consideration? This way, it won't be forgotten and can be addressed when it's more appropriate.

@overheadhunter overheadhunter merged commit d23d4d6 into develop Mar 21, 2024
7 checks passed
@overheadhunter overheadhunter deleted the feature/jdk-22 branch March 21, 2024 08:59
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.

2 participants