tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: RFC: Constant Database Support



On Sat, Mar 20, 2010 at 02:34:46PM +0000, Christos Zoulas wrote:
> - cdbr.h cpp protection inclusion symbol should match the filename.

Done.

> - I would sort the members in struct cdbr by size to avoid padding.

I'll move the base pointers ahead, but keep the rest as is. The small
padding after fd and the index_size is harmless and changing the order
would split the logical groups.

> - should that + 40 in cdbr_open be a sizeof(something)?

See cdbr->seed.

> - perhaps get_uintX should have a default case that aborts?

Can't happen without memory corruption. Wouldn't buy anything, I think.

> - cdbr_get does not always set errno.

It will set EIO on the failed consistency checks.

> - nor does cdbr_find.

Like cdbr_get.

> - why keep cdbr->fd open after you mmaped? why keep it in the structure at 
> all?

Dropped.

> - I would sort struct cdbw too.

It is naturally sorted on 32bit and 64bit platforms?

> - Should the cdbw routines set errno? They do sometimes (malloc failures).

IMO errno doesn't provide useful data in this case.

> - I would put the magic number in some private header so I don't have to 
> repeat
>   it in two places.

Not sure if this helps at all. At least the reader will have to deal
with old versions if there ever is a change.

Patch is updated.

Joerg


Home | Main Index | Thread Index | Old Index