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

Clean up rootless.h, add optional Xina compatibility #81

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

opa334
Copy link
Contributor

@opa334 opa334 commented Apr 22, 2023

What does this implement/fix? Explain your changes.

This removes the code checking whether THEOS_PACKAGE_INSTALL_PREFIX is define, as this is now always defined.
Additionally it introduces backwards support for xina with the define XINA_SUPPORT that a dev can set inside the Makefile if Xina support is desired. Note that this should only be set for rootful builds.

@opa334 opa334 changed the title Clean up rootless.h add optional Xina compatibility Clean up rootless.h, add optional Xina compatibility Apr 22, 2023
Copy link
Member

@leptos-null leptos-null left a comment

Choose a reason for hiding this comment

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

Nice solution to implementing ROOT_PATH_VAR in C.

This still has an issue in a situation such as

char *getName() {
    char *name = malloc(64);
    // ...
    return ROOT_PATH_VAR(name);
}

however I do not believe it is possible to handle a case like this without leaking memory in C.

It occurs to me that VAR variants of these macros should not be used at all (instead, only the literal parts of paths need to be adjusted), however I think that's a separate discussion.

@opa334
Copy link
Contributor Author

opa334 commented May 31, 2023

Yeah I mean you can't really expect the VAR macros to free anything, you always need to free stuff yourself. While yeah I do agree these macros don't have a big use case, they should still exist for completion.

@leptos-null leptos-null merged commit 56bfc22 into theos:master Jun 29, 2023
@opa334
Copy link
Contributor Author

opa334 commented Jun 29, 2023

Shit I forgot to address this

the char arrays need to be changed to alloca because otherwise the compiler sometimes overwrites them.

So char outPath[PATH_MAX]; needs to be changed to char *outPath = alloca(PATH_MAX)

@leptos-null
Copy link
Member

I see. Per my previous comments though, I think the _VAR variants should be removed. I think it's overall a lot of trouble (for the C string, dealing with stack versus heap; and for the Obj-C string, dealing with ARC or not), and for the very few times when this would actually be used, I feel that it's best for the author to write the code themselves using the THEOS_PACKAGE_INSTALL_PREFIX macro. I'm planning to open a PR to do this, so I'm not concerned presently.

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