search

ludocode / mpack

MPack - A C encoder/decoder for the MessagePack serialization format / msgpack.org[C]
MIT License
521 stars 82 forks source link

Build for kernel module #81

Closed mloy closed 2 years ago

mloy commented 3 years ago

This relates to: https://github.com/ludocode/mpack/issues/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.

ludocode commented 3 years ago

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 commented 3 years ago

Thank you very much. This sounds really good.

ludocode commented 2 years ago

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

mloy commented 2 years ago

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

ludocode commented 2 years ago

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.