Better documentation of variants/usage

[stefanct]

NB: I am a first-time user who checked out the repo less than an hour ago.

The current documentation leaves the reader quite in the dark regarding the different versions that a default build creates AFAICT. Currently, there are 10 different static libraries produced and only very few related bits are explained (e.g., that there exists a ThreadX implementation). What's more, the bit that explains how to "install" the library explains that one should link with -Lpath/to/libmemory.a -lmemory, i.e. libmemory.a. However, that apparently does only add the aligned_ functions(?) Therefore, when trying a typical example project that calls malloc_addblock() as explained in the actual Usage Section the user is greeted with undefined reference tomalloc_addblock'` and ponders why.

Apart from expanding the documentation it might be a good idea to add a default implementation of malloc_addblock() that is either just a nop, or fprints a warning at runtime (if not disabled by some build option or so)?

PS: make help shows a tests target. I think that should be just test

[phillipjohnston]

Thanks, I'll work on this.

For variants: would you still expect that in the README, or is it fine to break that out into a separate document linked from the README?

I'll look into the linking part as well - I'm guessing this is a carryover from before I had the "add aligned_malloc/free but use the system malloc" variant.

Apart from expanding the documentation it might be a good idea to add a default implementation of malloc_addblock() that is either just a nop, or fprints a warning at runtime (if not disabled by some build option or so)?

All the implementations have a malloc_addblock implementation that is related to that version, is this suggestion in reference to the linking problem?

e.g.: https://github.com/embeddedartistry/libmemory/blob/master/src/malloc_freelist.c#L195

[stefanct]

For variants: would you still expect that in the README, or is it fine to break that out into a separate document linked from the README?

As long as it is easily discoverable I don't really care if it is in another file. In general I personally prefer single-document READMEs because they are more easily searchable and references are easier to follow... but at some point it gets just too big. In the case of libmemory, as it currently is, I think leaving it as single page would be ok, but splitting it into README (features, status, overview, docs, contribution etc), BUILDING (fetching source, build configuration, building, testing), and USING (variants(?), integration/correct linking, usage) or similar would also work (for me anyway).

I'll look into the linking part as well - I'm guessing this is a carryover from before I had the "add aligned_malloc/free but use the system malloc" variant.

Yes, I would also think that this is a reminiscence of the past but it really threw me off.

Apart from expanding the documentation it might be a good idea to add a default implementation of malloc_addblock() that is either just a nop, or fprints a warning at runtime (if not disabled by some build option or so)?

All the implementations have a malloc_addblock implementation that is related to that version, is this suggestion in reference to the linking problem?

e.g.: https://github.com/embeddedartistry/libmemory/blob/master/src/malloc_freelist.c#L195

There is no malloc_addblock() in libmemory.a and thus one gets the missing reference error when following the current documentation, which talks a lot about that function but only of linking with libmemory.a. Of course in the case of libmemory.a in a hosted environment one does not strictly need that function and calling it makes not really sense - however, having an implementation anyway would make it possible to share the same code in the user application (and documentation examples, tests etc.) no matter which variant is linked eventually. One easy way to accomplish that would be to add something like:

diff --git a/src/posix_memalign.c b/src/posix_memalign.c
index 996a85e..fa30924 100644
--- a/src/posix_memalign.c
+++ b/src/posix_memalign.c
@@ -1,3 +1,4 @@
+#include <malloc.h>
 #include <aligned_malloc.h>
 #include <assert.h>
 #include <errno.h>
@@ -5,6 +6,10 @@

 #define IS_POWER_2(x) (!((x) & ((x)-1)))

+__attribute__((weak)) void malloc_addblock(__attribute__((unused)) void* addr, __attribute__((unused)) size_t size)
+{
+}
+
 int posix_memalign(void** __memptr, size_t __alignment, size_t __size)
 {
  int r = ENOMEM;

As I wrote initially, I would add a run-time warning there too to make sure the user understands what is going on (else this is easily missed I guess).

[phillipjohnston]

Ok, I see what you mean now. Thanks for the feedback!