AmendHub

jcs

/

subtext

/

amendments

/

127

bile: Revert in-place saves, keep 8k alignment, fix bug in bile_alloc

In bile_alloc, the initial last_pos is set to the header length, but
if there is only one object, we never add its length to last_pos.
This only happens when we have objects and then delete all of them,
leaving only the map object. Normally we'd either have 0 or 1+map.
Add canary to bile struct, verify it exists in every function so we
can check that we have a valid bile file and not a bogus pointer.

jcs made amendment 127 5 months ago
--- bile.c Mon Jun 6 09:43:38 2022 +++ bile.c Thu Jun 9 15:44:36 2022 @@ -32,6 +32,7 @@ short bile_read_map(struct bile *bile, short bile_write_map(struct bile *bile); size_t bile_xwriteat(struct bile *bile, const size_t pos, const void *data, const size_t len); +void bile_check_sanity(struct bile *bile); /* Public API */ @@ -68,8 +69,9 @@ bile_create(const Str255 filename, short vrefnum, cons if (_bile_error) return NULL; SetFPos(fh, fsFromStart, 0); - + bile = xmalloczero(sizeof(struct bile)); + memcpy(bile->magic, BILE_MAGIC, sizeof(bile->magic)); bile->vrefnum = vrefnum; bile->frefnum = fh; bile->map_ptr.type = BILE_TYPE_MAPPTR; @@ -141,6 +143,7 @@ bile_open(const Str255 filename, short vrefnum) SetFPos(fh, fsFromStart, 0); bile = xmalloczero(sizeof(struct bile)); + memcpy(bile->magic, BILE_MAGIC, sizeof(bile->magic)); bile->vrefnum = vrefnum; bile->frefnum = fh; memcpy(bile->filename, filename, sizeof(bile->filename)); @@ -194,6 +197,16 @@ open_bail: return NULL; } +void +bile_check_sanity(struct bile *bile) +{ + if (bile == NULL) + panic("bile_check_sanity: NULL bile"); + + if (memcmp(bile->magic, BILE_MAGIC, sizeof(bile->magic)) != 0) + panic("bile_check_sanity: bogus magic"); +} + struct bile * bile_open_recover_map(const Str255 filename, short vrefnum) { @@ -212,20 +225,17 @@ bile_open_recover_map(const Str255 filename, short vre } short -bile_autoflush(struct bile *bile, short val) -{ - bile->autoflush = val; -} - -short bile_flush(struct bile *bile, short and_vol) { IOParam pb; short ret; + bile_check_sanity(bile); + memset(&pb, 0, sizeof(pb)); pb.ioRefNum = bile->frefnum; - ret = PBFlushFile(&pb, true); + /* if we're not flushing the volume, write async */ + ret = PBFlushFile(&pb, false); if (ret != noErr) return ret; @@ -243,8 +253,7 @@ bile_flush(struct bile *bile, short and_vol) void bile_close(struct bile *bile) { - if (bile == NULL) - panic("bile_close: bogus bile"); + bile_check_sanity(bile); _bile_error = 0; @@ -259,6 +268,8 @@ bile_find(struct bile *bile, const OSType type, const struct bile_object *o, *ocopy; unsigned long n; + bile_check_sanity(bile); + o = bile_object_in_map(bile, type, id); if (o == NULL) return NULL; @@ -275,8 +286,7 @@ bile_count_by_type(struct bile *bile, const OSType typ struct bile_object *o; size_t n, count = 0; - if (bile == NULL) - panic("bile_count_by_type: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; @@ -296,6 +306,8 @@ bile_sorted_ids_by_type(struct bile *bile, const OSTyp size_t count = 0, size = 0, n, j, t; size_t *ids; + bile_check_sanity(bile); + for (n = 0; n < bile->nobjects; n++) { o = &bile->map[n - 1]; if (o->type != type) @@ -327,8 +339,7 @@ bile_get_nth_of_type(struct bile *bile, const size_t i struct bile_object *o, *ocopy; size_t n, count = 0; - if (bile == NULL) - panic("bile_get_nth_of_type: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; @@ -354,8 +365,7 @@ bile_next_id(struct bile *bile, const OSType type) struct bile_object *o; size_t n, id = 1; - if (bile == NULL) - panic("bile_next_id: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; @@ -375,8 +385,7 @@ bile_delete(struct bile *bile, const OSType type, cons struct bile_object *o; size_t pos, size, wsize; - if (bile == NULL) - panic("bile_delete: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; @@ -390,10 +399,6 @@ bile_delete(struct bile *bile, const OSType type, cons pos = o->pos; size = o->size + BILE_OBJECT_SIZE; - bile_write_map(bile); - if (_bile_error) - return -1; - _bile_error = bile->last_error = SetFPos(bile->frefnum, fsFromStart, pos); if (_bile_error) @@ -409,18 +414,22 @@ bile_delete(struct bile *bile, const OSType type, cons return -1; } + bile_write_map(bile); + if (_bile_error) + return -1; + return 0; } size_t bile_read_object(struct bile *bile, const struct bile_object *o, - char *data, const size_t len) + void *data, const size_t len) { struct bile_object verify; size_t rsize, wantlen, ret; - if (bile == NULL) - panic("bile_read_object: bogus bile"); + bile_check_sanity(bile); + if (o == NULL) panic("bile_read_object: NULL object passed"); if (data == NULL) @@ -496,45 +505,44 @@ bile_read_object(struct bile *bile, const struct bile_ size_t bile_read(struct bile *bile, const OSType type, const unsigned long id, - char *data, const size_t len) + void *data, const size_t len) { struct bile_object *o; size_t ret; - if (bile == NULL) - panic("bile_read: bogus bile"); + bile_check_sanity(bile); + if (data == NULL) panic("bile_read: NULL data pointer passed"); _bile_error = bile->last_error = 0; - o = bile_find(bile, type, id); + o = bile_object_in_map(bile, type, id); if (o == NULL) { _bile_error = bile->last_error = -1; return 0; } - ret = bile_read_object(bile, o, data, len); - free(o); - - return ret; + return bile_read_object(bile, o, data, len); } size_t bile_read_alloc(struct bile *bile, const OSType type, - const unsigned long id, char **data) + const unsigned long id, void *data_ptr) { struct bile_object *o; size_t ret; + char **data; + + bile_check_sanity(bile); - if (bile == NULL) - panic("bile_read: bogus bile"); - if (data == NULL) + if (data_ptr == NULL) panic("bile_read: NULL data pointer passed"); - + + data = (char **)data_ptr; _bile_error = bile->last_error = 0; - o = bile_find(bile, type, id); + o = bile_object_in_map(bile, type, id); if (o == NULL) { _bile_error = bile->last_error = -1; *data = NULL; @@ -543,7 +551,6 @@ bile_read_alloc(struct bile *bile, const OSType type, *data = xmalloczero(o->size); ret = bile_read_object(bile, o, *data, o->size); - free(o); return ret; } @@ -556,8 +563,8 @@ bile_write(struct bile *bile, const OSType type, const size_t wrote; short error; - if (bile == NULL) - panic("bile_write: bogus bile"); + bile_check_sanity(bile); + if (len == 0) panic("bile_write: zero len passed"); if (data == NULL) @@ -565,37 +572,24 @@ bile_write(struct bile *bile, const OSType type, const _bile_error = bile->last_error = 0; - old = bile_object_in_map(bile, type, id); - if (old != NULL && old->size != len) + if ((old = bile_object_in_map(bile, type, id)) != NULL) old->type = BILE_TYPE_PURGE; + + new_obj = bile_alloc(bile, type, id, len); - if (old == NULL || old->size != len) { - new_obj = bile_alloc(bile, type, id, len); - - wrote = bile_xwriteat(bile, new_obj->pos, new_obj, - BILE_OBJECT_SIZE); - if (wrote != BILE_OBJECT_SIZE || bile->last_error) - return 0; - wrote = bile_xwriteat(bile, new_obj->pos + BILE_OBJECT_SIZE, data, - len); - if (wrote != len || bile->last_error) - return 0; - - /* update old object with PURGE type */ - wrote = bile_xwriteat(bile, old->pos + BILE_OBJECT_SIZE, data, - len); + wrote = bile_xwriteat(bile, new_obj->pos, new_obj, BILE_OBJECT_SIZE); + if (wrote != BILE_OBJECT_SIZE || bile->last_error) + return 0; + wrote = bile_xwriteat(bile, new_obj->pos + BILE_OBJECT_SIZE, data, len); + if (wrote != len || bile->last_error) + return 0; - SetFPos(bile->frefnum, fsFromLEOF, 0); - GetFPos(bile->frefnum, &bile->file_size); + SetFPos(bile->frefnum, fsFromLEOF, 0); + GetFPos(bile->frefnum, &bile->file_size); - bile_write_map(bile); - if (bile->last_error) - return 0; - } else { - wrote = bile_xwriteat(bile, old->pos + BILE_OBJECT_SIZE, data, len); - if (bile->last_error) - return 0; - } + bile_write_map(bile); + if (bile->last_error) + return 0; return wrote; } @@ -603,17 +597,21 @@ bile_write(struct bile *bile, const OSType type, const short bile_marshall_object(struct bile *bile, const struct bile_object_field *fields, const size_t nfields, - void *object, char **ret, size_t *retsize) + void *object, void *ret_ptr, size_t *ret_size) { + char **ret; char *data, *ptr; size_t size = 0, fsize = 0, n; bool write = false; - if (ret == NULL) + bile_check_sanity(bile); + + if (ret_ptr == NULL) panic("bile_pack_object invalid ret"); + ret = (char **)ret_ptr; *ret = NULL; - *retsize = 0; + *ret_size = 0; iterate_fields: for (n = 0; n < nfields; n++) { @@ -661,7 +659,7 @@ iterate_fields: } *ret = data; - *retsize = size; + *ret_size = size; return 0; } @@ -669,15 +667,17 @@ iterate_fields: short bile_unmarshall_object(struct bile *bile, const struct bile_object_field *fields, const size_t nfields, - const char *data, const size_t size, void *object, bool deep) + const void *data, const size_t size, void *object, bool deep) { size_t off, fsize = 0, n; char *ptr, *dptr; + bile_check_sanity(bile); + for (off = 0, n = 0; n < nfields; n++) { if (fields[n].size < 0) { /* dynamically-sized field, read length */ - memcpy(&fsize, data + off, sizeof(fsize)); + memcpy(&fsize, (char *)data + off, sizeof(fsize)); off += sizeof(fsize); } else fsize = fields[n].size; @@ -697,7 +697,7 @@ bile_unmarshall_object(struct bile *bile, if (fields[n].size < 0 && !deep) memset(ptr, 0, sizeof(dptr)); else - memcpy(ptr, data + off, fsize); + memcpy(ptr, (char *)data + off, fsize); off += fsize; } @@ -713,8 +713,7 @@ bile_verify(struct bile *bile) size_t n, size; char data; - if (bile == NULL) - panic("bile_verify: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; @@ -738,8 +737,7 @@ bile_object_in_map(struct bile *bile, const OSType typ struct bile_object *o; size_t n; - if (bile == NULL) - panic("bile_find: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; @@ -761,14 +759,14 @@ bile_alloc(struct bile *bile, const OSType type, const size_t last_pos = BILE_HEADER_LEN; size_t n, map_pos; - if (bile == NULL) - panic("bile_alloc: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; /* find a last_pos we can use */ for (n = 0; n < bile->nobjects; n++) { - if (bile->map[n].pos - last_pos >= (size + BILE_OBJECT_SIZE)) + if (n > 0 && + bile->map[n].pos - last_pos >= (size + BILE_OBJECT_SIZE)) break; last_pos = bile->map[n].pos + BILE_OBJECT_SIZE + bile->map[n].size; } @@ -815,6 +813,8 @@ bile_read_map(struct bile *bile, struct bile_object *m size_t size; struct bile_object map_obj, *map; + bile_check_sanity(bile); + if (map_ptr->pos + map_ptr->size > bile->file_size) { warn("bile_read_map: map points to %lu + %lu, but file is only %lu", map_ptr->pos, map_ptr->size, bile->file_size); @@ -873,8 +873,7 @@ bile_write_map(struct bile *bile) size_t n; short ret; - if (bile == NULL) - panic("bile_write_map: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; @@ -926,6 +925,11 @@ bile_write_map(struct bile *bile) SetFPos(bile->frefnum, fsFromLEOF, 0); GetFPos(bile->frefnum, &bile->file_size); + if ((ret = bile_flush(bile, false)) != noErr) { + warn("bile_write_map: flush failed: %d", ret); + return -1; + } + /* successfully wrote new map, switch over */ free(bile->map); bile->nobjects = new_nobjects; @@ -948,7 +952,7 @@ bile_write_map(struct bile *bile) return -1; if ((ret = bile_flush(bile, false)) != noErr) { - warn("bile_write_map: flush failed: %d", ret); + warn("bile_write_map: final flush failed: %d", ret); return -1; } @@ -961,13 +965,14 @@ bile_xwriteat(struct bile *bile, const size_t pos, con { short error; size_t wsize, tsize; + long asize; - if (bile == NULL) - panic("bile_xwriteat: bogus bile"); + bile_check_sanity(bile); _bile_error = bile->last_error = 0; if (pos + len > bile->file_size) { + /* add new space aligning to BILE_ALLOCATE_SIZE */ tsize = pos + len; tsize += BILE_ALLOCATE_SIZE - (tsize % BILE_ALLOCATE_SIZE); _bile_error = SetEOF(bile->frefnum, tsize); --- bile.h Mon Jun 6 09:43:43 2022 +++ bile.h Wed Jun 8 11:26:04 2022 @@ -39,10 +39,8 @@ * [ object[0] size - long ] * [ object[0] type - long ] * [ object[0] id - long ] - * [ object[0] data ] * [ object[1] start ] * [ .. ] - * [ object[1] data ] * [ map object - (pointed to by map pointer position) ] * [ map position - long ] * [ map size - long ] @@ -86,7 +84,7 @@ struct bile { size_t file_size; struct bile_object *map; /* array of bile_objects */ size_t nobjects; - short autoflush; + char magic[5]; }; struct bile_object_field { @@ -103,7 +101,6 @@ struct bile * bile_open(const Str255 filename, short struct bile * bile_open_recover_map(const Str255 filename, short vrefnum); short bile_flush(struct bile *bile, short and_vol); -short bile_autoflush(struct bile *bile, short val); void bile_close(struct bile *bile); struct bile_object * bile_find(struct bile *bile, const OSType type, @@ -118,14 +115,14 @@ size_t bile_next_id(struct bile *bile, const OSTyp short bile_delete(struct bile *bile, const OSType type, const unsigned long id); size_t bile_read_object(struct bile *bile, - const struct bile_object *o, char *data, + const struct bile_object *o, void *data, const size_t len); size_t bile_read(struct bile *bile, const OSType type, - const unsigned long id, char *data, + const unsigned long id, void *data, const size_t len); size_t bile_read_alloc(struct bile *bile, const OSType type, const unsigned long id, - char **data); + void *data_ptr); size_t bile_write(struct bile *bile, OSType type, const unsigned long id, const void *data, const size_t len); @@ -133,11 +130,11 @@ short bile_verify(struct bile *bile); short bile_marshall_object(struct bile *bile, const struct bile_object_field *fields, - const size_t nfields, void *object, char **ret, - size_t *retsize); + const size_t nfields, void *object, + void *ret_ptr, size_t *ret_size); short bile_unmarshall_object(struct bile *bile, const struct bile_object_field *fields, - const size_t nfields, const char *data, + const size_t nfields, const void *data, const size_t size, void *object, bool deep); #endif