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

options/posix: port musl regex engine #511

Merged
merged 2 commits into from
Jun 22, 2022
Merged

Conversation

64
Copy link
Member

@64 64 commented Jun 19, 2022

Fixes #98.

@64 64 added the affects ABI label Jun 19, 2022
@64 64 force-pushed the regcomp branch 2 times, most recently from dd95cc3 to 32a59a3 Compare June 19, 2022 15:58
@64 64 linked an issue Jun 19, 2022 that may be closed by this pull request
Copy link
Member

@ArsenArsen ArsenArsen left a comment

Choose a reason for hiding this comment

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

Skimming through the code, this seems fine.

#include <assert.h>

int main(void) {
char *testString = "mlibc is the best best best libc";
Copy link
Member

Choose a reason for hiding this comment

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

singing hard praise for a WIP project :P

@@ -305,3 +306,6 @@ void globfree(glob_t *g)
g->gl_pathc = 0;
g->gl_pathv = NULL;
}

// weak_alias(glob, glob64);
// weak_alias(globfree, globfree64);
Copy link
Member

Choose a reason for hiding this comment

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

do we need these?

'musl-generic-regex/tre-mem.c',
pic: true,
include_directories: libc_include_dirs,
c_args: ['-Wno-unused', '-Wno-implicit', '-Wno-parentheses', '-Wno-sign-compare', '-Wno-attributes', '-Wno-unknown-pragmas', '-Wno-implicit-fallthrough']
Copy link
Member

Choose a reason for hiding this comment

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

have we tried fixing these?

Copy link
Member

@Geertiebear Geertiebear left a comment

Choose a reason for hiding this comment

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

Not much to say on this PR, besides that fixing the warnings would be nice, if it's not too much work.

@@ -18,7 +18,7 @@
#include <stdlib.h>
#include <wchar.h>
#include <wctype.h>
//#include "locale_impl.h"
// #include "locale_impl.h"
Copy link
Member

Choose a reason for hiding this comment

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

Why not just simply remove this?

@64
Copy link
Member Author

64 commented Jun 20, 2022

Repeating what I said on Discord: I've commented out lines / not fixed warnings so that pulling future changes from musl will be marginally more straightforward. I don't think fixing the warnings is a good idea; it'll just create more pain.

@avdgrinten
Copy link
Member

Yes, fixing the warnings would probably be quite annoying.

I wonder if we should simply add musl as a subproject and write a Meson generator that copies the files and patches them.

@64
Copy link
Member Author

64 commented Jun 20, 2022

Yes, fixing the warnings would probably be quite annoying.

I wonder if we should simply add musl as a subproject and write a Meson generator that copies the files and patches them.

I'm not against that, but we should probably do that in a separate PR

@64 64 force-pushed the regcomp branch 2 times, most recently from dc903cc to 56efca3 Compare June 21, 2022 12:28
@ArsenArsen
Copy link
Member

Probably not, fetching musl at build time seems silly.

@Geertiebear Geertiebear merged commit c771db9 into managarm:abi-break Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

regcomp() unimplemented
5 participants