Skip to content

Conversation

mloy
Copy link

@mloy mloy commented Sep 12, 2020

This relates to: #80

The request contains everything I need to change to have it compiled withing a linux kernel module. Commit
c92c2b3 looks not that nice to me. I think this one needs some refurbishment.

After undefining MPACK_STDLIB and MPACK_STDIO, the following has to be
changed.

-Replace some header that are not known in kernel.
-__attribute__((noinline)) makes problems (This I do not understand).
@ludocode
Copy link
Owner

Thanks. It looks like the changes are doable. I had originally resisted adding gnu89 support; see the discussion in #68. Maybe it's time to revisit that. I want to support gnu89 on another project I'm getting ready to open source, and I'm rebuilding the unit test buildsystem for MPack (again) in the unit-test-cleanup branch, so I think it's a good time to implement this.

Side note though, is it not possible to build kernel modules as C99/gnu99? I know the kernel proper has to be gnu89, but all modules do as well? Aren't people writing modules in Rust these days?

For the loop initial declarations, there are a lot more to fix than the one you have here. #68 has most of the ones in the MPack library, but most of them are in the unit test suite. I'd like to add a gnu89 build to the unit test suite to make sure this doesn't break again, so that will mean fixing the unit tests as well. (Of course it's possible to build the library as gnu89 and the unit tests as C99, but that would probably complicate the buildsystem too much.)

The kernel having a macro on the word "current" is unfortunate. What you have here will work, although I might want to rename it to "position" or something. We should also poison it in the unit tests to make sure it doesn't get used again.

For noinline, probably the kernel has a macro on that as well; I'm guessing __attribute__((__noinline__)) would work. We should probably use double underscores for all attributes since it seems likely that more projects would have a macro on that word, and we should poison it in the unit tests as well.

As for the types, it's unfortunate that the kernel doesn't define the standard macros. Maybe we should be wrapping these, e.g. MPACK_UINT32_MAX, that way we won't conflict with other code that defines them. If not we should at least use #ifndef.

I think I can merge this and #68 as-is and then clean things up after. Let me just do a bit more testing first.

@mloy
Copy link
Author

mloy commented Sep 14, 2020

Thank you very much. This sounds really good.

@ludocode
Copy link
Owner

ludocode commented Aug 3, 2021

Building in the Linux kernel is now supported. I merged f9ec219 from this PR and made a ton of other changes (wrapped all the headers and limits macros, added an option to disable floats, cleaned up stack usage in the unit tests, etc.)

I will not merge c92c2b3 since including the Linux headers directly would make MPack a derivative work of the Linux kernel requiring dual-licensing it under the GPL. Instead an mpack-config.h can be used to include the Linux headers. The sample mpack-config.h as well as a shim to build the MPack unit test suite as a Linux kernel module are in a separate project here: https://github.com/ludocode/mpack-linux-kernel

@ludocode ludocode closed this Aug 3, 2021
@mloy
Copy link
Author

mloy commented Aug 3, 2021

This is really nice! Are you planning to make a release?

@ludocode
Copy link
Owner

ludocode commented Aug 4, 2021

Yeah, actually I was hoping to get it done before this week. I ran out of time and I just started a new unrelated contract so I'm not sure when I'll have time again. Probably one of the coming weekends I'll get around to it.

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