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 over 2 years 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