Minor freelist stuff

[stefanct]

Hi,

I have made some minor changes to the freelist implementation and its tests. The tests could be further enhanced by varying the sizes and also checking for non-overlapping blocks but I don't intend to do that.

The last change might be controversial. It changes the scope of struct ll_head free_list to be global instead of limited to compilation unit. I require this to be able to manipulate the metadata outside of the implementation for a research project. I was looking for alternatives to change the visibility of this kind of variables but it's basically impossible to do this - not even by external tools re-writing static libraries/object files. One might be able to implement such a tool but it's certainly not trivial. Newlib's metadata is usually also global (depending on how it's compiled) but that's of course a weak argument. If you see problems with simply making it global maybe we could make it configurable?

[stefanct]

The CI reports are not publicly visible BTW.

[phillipjohnston]

Thank you very much for the PR.

• I've filed #83 to track the test improvements, I agree with you.
• CI reports are now public
  • I see in this instance clang-format step is passing even though it detects required changes, I will fix that.
• If you have clang-format installed, you can run make format to apply the formatting changes directly. Otherwise you can apply this patch file.

As to the controversial change - I am not inclined to make such a change... Outside of your research project, I can't see how it would be a desirable property of the library for an application to modify the metadata of the freelist directly. Given that it seems to be a one-off change for a specific project, would it be possible to live in a fork? If there is a reason you think it would be a better to live here, I'd be happy to hear it.

Should we go down the path of making the freelist metadata available in the library, I would prefer the approach below. Would this work for your needs?

• Keep the list declaration static
• Create a public function that will return the pointer to the freelist, but condition the inclusion of that function on a #define
• Add a build option that can be used to control the definition of that function

[stefanct]

I have updated the branch with the format fix and removed the commit that removes the static qualifier.

Regarding the scope of the list: I totally understand your reluctance in this matter. I can tell you how I use it as I think in some weird situations this could actually be a use case to consider: I take snapshots of the metadata and user data at some points in the execution to make it possible to easily go back to this later.

Using a function to get access to static variables is the common solution when designing a properly encapsulated relationship. I think in this case here it is unnecessary and to some extent counter-productive: it gives potential users the impression that this is something one could use casually when in fact it is rather a hack that should only be used if one really knows what they are doing ;) Joking aside, I think that only providing a way to optionally make the necessary variables non-static is less invasive, easier to maintain (for the library owner), and just as good for the library user. I am fine with all 3 options though, leaving everything as is, introducing an optional getter function, or making it optionally non-static.

[stefanct]

Apparently my brain thought about this a bit more during sleep... :)

A maybe less obscure use case would be a monitoring task that regularly iterates over the metadata to gather statistics of heap usage like total free space, largest free block etc.

One way around the whole static problem would be to let the user provide the memory for the metadata themselves. That would also make it easier to use libmemory in a multitasking environment without the need for locks (but of course would require some API changes/additions).

[phillipjohnston]

After also sleeping on it, I will make the static/non-static configuration option. I think the idea about threads being able to supply their own freelist memory is a great idea that is worth exploring, but I would make a new implementation for that.