Re: [PATCH 3/8] Add memory pool library

> Ramkumar Ramachandra wrote:
> 
> > void pre_commit(void);
> > 
> > 	Write the pool to file.
> 
> Except as a proof of concept, this is the wrong API to have.  The problem
> is that the caller cannot choose the filename, so it ends up being a .bin
> file in the current directory, wherever that is.
> 
> The log message leaves out a subtlety: this also increases the
> ‘committed’ value, and bookkeeping for that might be useful to some
> callers.
> 
> In other words:
> 
> > +static MAYBE_UNUSED void pre##_init(void) \
> > +{ \
> > +	struct stat st; \
> > +	pre##_pool.file = fopen(#pre ".bin", "a+"); \
> > +	rewind(pre##_pool.file); \
> > +	fstat(fileno(pre##_pool.file), &st); \
> > +	pre##_pool.size = st.st_size / sizeof(obj_t); \
> > +	pre##_pool.committed = pre##_pool.size; \
> > +	pre##_pool.capacity = pre##_pool.size * 2; \
> > +	if (pre##_pool.capacity < initial_capacity) \
> > +		pre##_pool.capacity = initial_capacity; \
> > +	pre##_pool.base = malloc(pre##_pool.capacity * sizeof(obj_t)); \
> > +	fread(pre##_pool.base, sizeof(obj_t), pre##_pool.size, pre##_pool.file); \
> > +} \
> 
> If you just want something working, I’d suggest stubbing this out:
> 
>  static MAYBE_UNUSED void pre##_init(void) \
>  { \
>  } \
> 
> It even almost makes sense as API: the _init function does all
> initialization tasks required, which is to say, none.  (The {0, ...}
> initializer already has taken care of setting all fields to 0).
> 
> > +static MAYBE_UNUSED void pre##_commit(void) \
> > +{ \
> > +	pre##_pool.committed += fwrite(pre##_pool.base + pre##_pool.committed, \
> > +		sizeof(obj_t), pre##_pool.size - pre##_pool.committed, \
> > +		pre##_pool.file); \
> > +} \
> 
> This can be simplified
> 
>  static MAYBE_UNUSED void pre##_commit(void) \
>  { \
> 	pre##_pool.committed = pre##_pool.size; \
>  } \
> 
> In other words, maybe something like this on top?  This includes the
> vestigal _init() function which really should not be there (it is
> confusing that some callers use it and others don’t).  I did not
> spend much time on it because in the end I suspect we might throw
> obj_pool away anyway.