search

yaml / libyaml

Canonical source repository for LibYAML
http://pyyaml.org/wiki/LibYAML
MIT License
953 stars 317 forks source link

Feature Request: User-specified allocator #178

Open mkmckenz opened 4 years ago

mkmckenz commented 4 years ago

We desire to prevent libyaml from using malloc/free directly, and instead have it call user-specified functions to allocate and release memory. It seems there was a past pull request to add this kind of feature, but it never passed/merged. I would like to rekindle that feature and am interested to know if you would anticipate any gotchas.

Old pull request, for reference: https://github.com/yaml/libyaml/pull/62

adimeco commented 4 years ago

Same feedback here. I've been waiting for a while the PR #62 to be merged but it seems to be not happening, and there is no clear reason for that.

perlpunk commented 4 years ago

For me the reason has been that I

:)

Would be glad for a bit more explanation, and maybe @nitzmahone or someone else knowing more C than me could help reviewing?

mkmckenz commented 4 years ago

A couple possible use-cases for consideration: a) In Real-Time software it is common that use of malloc/free/new/delete are prohibited after initialization is complete in order to promote deterministic execution. Granted, parsing text is a somewhat non-deterministic activity in itself, so use of custom allocators is necessary but not strictly sufficient for real-time safety. b) In a bare-metal embedded software situation there may not be a system heap at all, in which case custom allocators become necessary. Linking a custom malloc/free implementation in just for libyaml to use is possible, but potentially problematic in unit testing and other integration scenarios. Having support in libyaml for custom allocators would help avoid such collision problems.

adimeco commented 4 years ago

Indeed, the purpose of the PR #62 is to allow the user to manage memory allocations his own way, and to not use the default malloc/free implementation. There are many reasons for that: performances, debugging features, containing memory fragmentation, seggregating allocations, etc....

What you expect from a good C library is to focus on his own purpose (in our case, serializing yaml documents) and not to take any assumption on how system resources need to be managed (accessing files, launching threads, allocating memory, etc...). This kind of consideration is very "application-dependant" and should be left to the user. It is perfectly fine to provide a default behavior, but not having any way to change that behavior is an issue. Interestingly, libYaml understands that regarding file accesses (see yaml_parser_set_input/yaml_emitter_set_output functions) but fails to do the same for memory allocations.

Most of the well known C/C++ libraries provide a way to hook memory allocation functions. As an example, you can have a look at zlib for instance, or libpng, or libJPEG, it's really common and not having this feature is kind of weird for this type of library. I presume that libYaml users that need to do so make their own change locally, I personally apply a git patch every time I want to update to the lastes libYaml version.

Hope that helps,

Cheers, and thanks for the good work you guys are doing on this lib!

ipilcher commented 3 years ago

I'll echo what @adimeco said above. This is definitely a valid use case.

I took a quick look at the PR, and I didn't see any obvious issues (assuming that all memory [de-]allocations are already going through the yaml_ wrapper functions, which I didn't check). With that caveat, I would encourage you to merge the PR.