Skip to content

Conversation

@hasumikin
Copy link
Contributor

This is to know if the memory map interface is available in the target platform by using _POSIX_MAPPED_FILES and _WIN32

ref #2217

@kddnewton
Copy link
Collaborator

@hasumikin I think _POSIX_MAPPED_FILES is defined in unistd.h, so you probably need to include that before doing the check.

@hasumikin
Copy link
Contributor Author

@kddnewton
Thank you.
My problem was fixed but another static check error was revealed that I couldn't figure out how to resolve :(

@kddnewton
Copy link
Collaborator

Thanks @hasumikin you should be able to rebase now and it should be fixed.

My feedback on the PR, is that I think we should keep PM_STRING_MAPPED around, even if PRISM_HAS_MMAP is false. Maybe we should rename it to PM_STRING_FILE or something, but that way we could malloc and read the file into a const uint8_t * the same way you would mmap it.

@kddnewton
Copy link
Collaborator

I'm also happy to make those changes after we merge this if you'd prefer. No problem! 😀

@hasumikin
Copy link
Contributor Author

hasumikin commented Feb 19, 2024

@kddnewton

My feedback on the PR, is that I think we should keep PM_STRING_MAPPED around, even if PRISM_HAS_MMAP is false. Maybe we should rename it to PM_STRING_FILE or something, but that way we could malloc and read the file into a const uint8_t * the same way you would mmap it.

If I didn't get you wrong, we'd better create one in addition to PM_STRING_MAPPED and not include _FILE because (from the embedded system point of view,) we can not assume we have a filesystem.

How about PM_STRING_GENERAL for instance?

/* pm_string.h */
typedef struct {
 (......)
    /** The type of the string. This field determines how the string should be freed. */
    enum {
     (......)

        /** This string owns its memory, and should be freed using `pm_string_free`. */
        PM_STRING_OWNED,

        /** This is a general string, and should be taken care of by the user. */
        PM_STRING_GENERAL,    // 👈

#ifdef PRISM_HAS_MMAP
        /** This string is a memory-mapped file, and should be freed using `pm_string_free`. */
        PM_STRING_MAPPED
#endif
    } type;
} pm_string_t;

@kddnewton
Copy link
Collaborator

Actually I realize now it should simply return a PM_STRING_OWNED if it doesn't have mmap, because that's effectively what it is. I'll merge this as-is and see what it takes to do that.

@kddnewton
Copy link
Collaborator

Thank you for the contribution!

@kddnewton kddnewton merged commit d0d0a9e into ruby:main Feb 20, 2024
@hasumikin hasumikin deleted the prism_has_mmap branch February 21, 2024 12:17
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