diff --git a/TODO b/TODO index a05900d42..298730f5f 100644 --- a/TODO +++ b/TODO @@ -16,8 +16,6 @@ API issues to fix before 1.0: - Add hb-cairo glue -- Fix blob, remove mutex, etc. - - Add sanitize API - Add glib GBoxedType stuff diff --git a/src/hb-blob.cc b/src/hb-blob.cc index b903acc12..cdb74965d 100644 --- a/src/hb-blob.cc +++ b/src/hb-blob.cc @@ -28,7 +28,6 @@ #include "hb-blob.h" #include "hb-object-private.hh" -#include "hb-mutex-private.hh" #ifdef HAVE_SYS_MMAN_H #ifdef HAVE_UNISTD_H @@ -51,15 +50,11 @@ HB_BEGIN_DECLS struct _hb_blob_t { hb_object_header_t header; - unsigned int length; - - hb_mutex_t lock; - /* the rest are protected by lock */ - - unsigned int lock_count; - hb_memory_mode_t mode; + bool immutable; const char *data; + unsigned int length; + hb_memory_mode_t mode; void *user_data; hb_destroy_func_t destroy; @@ -68,19 +63,19 @@ struct _hb_blob_t { static hb_blob_t _hb_blob_nil = { HB_OBJECT_HEADER_STATIC, + TRUE, /* immutable */ + 0, /* length */ - - HB_MUTEX_INIT, /* lock */ - - 0, /* lock_count */ - HB_MEMORY_MODE_READONLY, /* mode */ - NULL, /* data */ + HB_MEMORY_MODE_READONLY, /* mode */ NULL, /* user_data */ NULL /* destroy */ }; + +static bool _try_writable (hb_blob_t *blob); + static void _hb_blob_destroy_user_data (hb_blob_t *blob) { @@ -91,13 +86,6 @@ _hb_blob_destroy_user_data (hb_blob_t *blob) } } -static void -_hb_blob_unlock_and_destroy (hb_blob_t *blob) -{ - hb_blob_unlock (blob); - hb_blob_destroy (blob); -} - hb_blob_t * hb_blob_create (const char *data, unsigned int length, @@ -113,9 +101,6 @@ hb_blob_create (const char *data, return &_hb_blob_nil; } - hb_mutex_init (&blob->lock); - blob->lock_count = 0; - blob->data = data; blob->length = length; blob->mode = mode; @@ -125,7 +110,7 @@ hb_blob_create (const char *data, if (blob->mode == HB_MEMORY_MODE_DUPLICATE) { blob->mode = HB_MEMORY_MODE_READONLY; - if (!hb_blob_try_writable (blob)) { + if (!_try_writable (blob)) { hb_blob_destroy (blob); return &_hb_blob_nil; } @@ -140,20 +125,17 @@ hb_blob_create_sub_blob (hb_blob_t *parent, unsigned int length) { hb_blob_t *blob; - const char *pdata; if (!length || offset >= parent->length || !(blob = hb_object_create ())) return &_hb_blob_nil; - pdata = hb_blob_lock (parent); + hb_blob_make_immutable (parent); - hb_mutex_lock (&parent->lock); - blob = hb_blob_create (pdata + offset, + blob = hb_blob_create (parent->data + offset, MIN (length, parent->length - offset), parent->mode, hb_blob_reference (parent), - (hb_destroy_func_t) _hb_blob_unlock_and_destroy); - hb_mutex_unlock (&parent->lock); + (hb_destroy_func_t) hb_blob_destroy); return blob; } @@ -176,7 +158,6 @@ hb_blob_destroy (hb_blob_t *blob) if (!hb_object_destroy (blob)) return; _hb_blob_destroy_user_data (blob); - hb_mutex_free (&blob->lock); free (blob); } @@ -198,6 +179,22 @@ hb_blob_get_user_data (hb_blob_t *blob, } +void +hb_blob_make_immutable (hb_blob_t *blob) +{ + if (hb_object_is_inert (blob)) + return; + + blob->immutable = TRUE; +} + +hb_bool_t +hb_blob_is_immutable (hb_blob_t *blob) +{ + return blob->immutable; +} + + unsigned int hb_blob_get_length (hb_blob_t *blob) { @@ -205,62 +202,33 @@ hb_blob_get_length (hb_blob_t *blob) } const char * -hb_blob_lock (hb_blob_t *blob) +hb_blob_get_data (hb_blob_t *blob, unsigned int *length) { - if (hb_object_is_inert (blob)) - return NULL; - - hb_mutex_lock (&blob->lock); - - (void) (HB_DEBUG_BLOB && - fprintf (stderr, "%p %s (%d) -> %p\n", blob, HB_FUNC, - blob->lock_count, blob->data)); - - blob->lock_count++; - - hb_mutex_unlock (&blob->lock); + if (length) + *length = blob->length; return blob->data; } -void -hb_blob_unlock (hb_blob_t *blob) +char * +hb_blob_get_data_writable (hb_blob_t *blob, unsigned int *length) { - if (hb_object_is_inert (blob)) - return; + if (!_try_writable (blob)) { + if (length) + *length = 0; - hb_mutex_lock (&blob->lock); + return NULL; + } - (void) (HB_DEBUG_BLOB && - fprintf (stderr, "%p %s (%d) -> %p\n", blob, HB_FUNC, - blob->lock_count, blob->data)); + if (length) + *length = blob->length; - assert (blob->lock_count > 0); - blob->lock_count--; - - hb_mutex_unlock (&blob->lock); -} - -hb_bool_t -hb_blob_is_writable (hb_blob_t *blob) -{ - hb_memory_mode_t mode; - - if (hb_object_is_inert (blob)) - return FALSE; - - hb_mutex_lock (&blob->lock); - - mode = blob->mode; - - hb_mutex_unlock (&blob->lock); - - return mode == HB_MEMORY_MODE_WRITABLE; + return const_cast (blob->data); } static hb_bool_t -_try_make_writable_inplace_unix_locked (hb_blob_t *blob) +_try_make_writable_inplace_unix (hb_blob_t *blob) { #if defined(HAVE_SYS_MMAN_H) && defined(HAVE_MPROTECT) uintptr_t pagesize = -1, mask, length; @@ -295,6 +263,8 @@ _try_make_writable_inplace_unix_locked (hb_blob_t *blob) return FALSE; } + blob->mode = HB_MEMORY_MODE_WRITABLE; + (void) (HB_DEBUG_BLOB && fprintf (stderr, "%p %s: successfully made [%p..%p] (%lu bytes) writable\n", blob, HB_FUNC, @@ -305,67 +275,59 @@ _try_make_writable_inplace_unix_locked (hb_blob_t *blob) #endif } -static void -try_writable_inplace_locked (hb_blob_t *blob) +static bool +_try_writable_inplace (hb_blob_t *blob) { (void) (HB_DEBUG_BLOB && - fprintf (stderr, "%p %s: making writable\n", blob, HB_FUNC)); + fprintf (stderr, "%p %s: making writable inplace\n", blob, HB_FUNC)); - if (_try_make_writable_inplace_unix_locked (blob)) { - (void) (HB_DEBUG_BLOB && - fprintf (stderr, "%p %s: making writable -> succeeded\n", blob, HB_FUNC)); - blob->mode = HB_MEMORY_MODE_WRITABLE; - } else { - (void) (HB_DEBUG_BLOB && - fprintf (stderr, "%p %s: making writable -> FAILED\n", blob, HB_FUNC)); - /* Failed to make writable inplace, mark that */ - blob->mode = HB_MEMORY_MODE_READONLY; - } + if (_try_make_writable_inplace_unix (blob)) + return TRUE; + + (void) (HB_DEBUG_BLOB && + fprintf (stderr, "%p %s: making writable -> FAILED\n", blob, HB_FUNC)); + + /* Failed to make writable inplace, mark that */ + blob->mode = HB_MEMORY_MODE_READONLY; + return FALSE; } -hb_bool_t -hb_blob_try_writable (hb_blob_t *blob) +static bool +_try_writable (hb_blob_t *blob) { - hb_memory_mode_t mode; - - if (hb_object_is_inert (blob)) + if (blob->immutable) return FALSE; - hb_mutex_lock (&blob->lock); + if (blob->mode == HB_MEMORY_MODE_WRITABLE) + return TRUE; - if (blob->mode == HB_MEMORY_MODE_READONLY_MAY_MAKE_WRITABLE) - try_writable_inplace_locked (blob); + if (blob->mode == HB_MEMORY_MODE_READONLY_MAY_MAKE_WRITABLE && _try_writable_inplace (blob)) + return TRUE; - if (blob->mode == HB_MEMORY_MODE_READONLY) - { - char *new_data; + if (blob->mode == HB_MEMORY_MODE_WRITABLE) + return TRUE; - (void) (HB_DEBUG_BLOB && - fprintf (stderr, "%p %s (%d) -> %p\n", blob, HB_FUNC, - blob->lock_count, blob->data)); - if (blob->lock_count) - goto done; + (void) (HB_DEBUG_BLOB && + fprintf (stderr, "%p %s -> %p\n", blob, HB_FUNC, blob->data)); - new_data = (char *) malloc (blob->length); - if (new_data) { - (void) (HB_DEBUG_BLOB && - fprintf (stderr, "%p %s: dupped successfully -> %p\n", blob, HB_FUNC, blob->data)); - memcpy (new_data, blob->data, blob->length); - _hb_blob_destroy_user_data (blob); - blob->mode = HB_MEMORY_MODE_WRITABLE; - blob->data = new_data; - blob->user_data = new_data; - blob->destroy = free; - } - } + char *new_data; -done: - mode = blob->mode; + new_data = (char *) malloc (blob->length); + if (unlikely (!new_data)) + return FALSE; - hb_mutex_unlock (&blob->lock); + (void) (HB_DEBUG_BLOB && + fprintf (stderr, "%p %s: dupped successfully -> %p\n", blob, HB_FUNC, blob->data)); - return mode == HB_MEMORY_MODE_WRITABLE; + memcpy (new_data, blob->data, blob->length); + _hb_blob_destroy_user_data (blob); + blob->mode = HB_MEMORY_MODE_WRITABLE; + blob->data = new_data; + blob->user_data = new_data; + blob->destroy = free; + + return TRUE; } diff --git a/src/hb-blob.h b/src/hb-blob.h index 3ec35a843..f2eaae967 100644 --- a/src/hb-blob.h +++ b/src/hb-blob.h @@ -74,20 +74,21 @@ hb_blob_get_user_data (hb_blob_t *blob, hb_user_data_key_t *key); +void +hb_blob_make_immutable (hb_blob_t *blob); + +hb_bool_t +hb_blob_is_immutable (hb_blob_t *blob); + + unsigned int hb_blob_get_length (hb_blob_t *blob); const char * -hb_blob_lock (hb_blob_t *blob); +hb_blob_get_data (hb_blob_t *blob, unsigned int *length); -void -hb_blob_unlock (hb_blob_t *blob); - -hb_bool_t -hb_blob_is_writable (hb_blob_t *blob); - -hb_bool_t -hb_blob_try_writable (hb_blob_t *blob); +char * +hb_blob_get_data_writable (hb_blob_t *blob, unsigned int *length); HB_END_DECLS diff --git a/src/hb-font.cc b/src/hb-font.cc index e41ceec34..1e7734b59 100644 --- a/src/hb-font.cc +++ b/src/hb-font.cc @@ -361,8 +361,6 @@ _hb_face_for_data_get_table (hb_tag_t tag, void *user_data) hb_blob_t *blob = hb_blob_create_sub_blob (data->blob, table.offset, table.length); - hb_blob_unlock (data->blob); - return blob; } diff --git a/src/hb-open-type-private.hh b/src/hb-open-type-private.hh index 20df95ba4..e16eddd59 100644 --- a/src/hb-open-type-private.hh +++ b/src/hb-open-type-private.hh @@ -186,9 +186,13 @@ struct hb_sanitize_context_t inline void init (hb_blob_t *blob) { this->blob = hb_blob_reference (blob); - this->start = hb_blob_lock (blob); + this->writable = false; + } + + inline void setup (void) + { + this->start = hb_blob_get_data (blob, NULL); this->end = this->start + hb_blob_get_length (blob); - this->writable = hb_blob_is_writable (blob); this->edit_count = 0; this->debug_depth = 0; @@ -204,7 +208,6 @@ struct hb_sanitize_context_t fprintf (stderr, "sanitize %p fini [%p..%p] %u edit requests\n", this->blob, this->start, this->end, this->edit_count)); - hb_blob_unlock (this->blob); hb_blob_destroy (this->blob); this->blob = NULL; this->start = this->end = NULL; @@ -286,11 +289,13 @@ struct Sanitizer /* TODO is_sane() stuff */ + c->init (blob); + retry: (void) (HB_DEBUG_SANITIZE && fprintf (stderr, "Sanitizer %p start %s\n", blob, HB_FUNC)); - c->init (blob); + c->setup (); if (unlikely (!c->start)) { c->finish (); @@ -320,11 +325,17 @@ struct Sanitizer } else { unsigned int edit_count = c->edit_count; c->finish (); - if (edit_count && !hb_blob_is_writable (blob) && hb_blob_try_writable (blob)) { - /* ok, we made it writable by relocating. try again */ - (void) (HB_DEBUG_SANITIZE && - fprintf (stderr, "Sanitizer %p retry %s\n", blob, HB_FUNC)); - goto retry; + if (edit_count && !c->writable) { + c->start = hb_blob_get_data_writable (blob, NULL); + c->end = c->start + hb_blob_get_length (blob); + + if (c->start) { + c->writable = true; + /* ok, we made it writable by relocating. try again */ + (void) (HB_DEBUG_SANITIZE && + fprintf (stderr, "Sanitizer %p retry %s\n", blob, HB_FUNC)); + goto retry; + } } } @@ -339,7 +350,8 @@ struct Sanitizer } static const Type* lock_instance (hb_blob_t *blob) { - const char *base = hb_blob_lock (blob); + hb_blob_make_immutable (blob); + const char *base = hb_blob_get_data (blob, NULL); return unlikely (!base) ? &Null(Type) : CastP (base); } }; diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index 7383e9f32..7e1e96689 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -45,7 +45,7 @@ HB_BEGIN_DECLS hb_ot_layout_t * _hb_ot_layout_create (hb_face_t *face) { - /* Remove this object altogether */ + /* TODO Remove this object altogether */ hb_ot_layout_t *layout = (hb_ot_layout_t *) calloc (1, sizeof (hb_ot_layout_t)); layout->gdef_blob = Sanitizer::sanitize (hb_face_reference_table (face, HB_OT_TAG_GDEF)); @@ -66,11 +66,6 @@ _hb_ot_layout_create (hb_face_t *face) void _hb_ot_layout_destroy (hb_ot_layout_t *layout) { - hb_blob_unlock (layout->gdef_blob); - hb_blob_unlock (layout->gsub_blob); - hb_blob_unlock (layout->gpos_blob); - hb_blob_unlock (layout->head_blob); - hb_blob_destroy (layout->gdef_blob); hb_blob_destroy (layout->gsub_blob); hb_blob_destroy (layout->gpos_blob); diff --git a/test/test-object.c b/test/test-object.c index 3662d3000..4936172a8 100644 --- a/test/test-object.c +++ b/test/test-object.c @@ -159,10 +159,10 @@ typedef struct { } static const object_t objects[] = { - OBJECT_WITHOUT_IMMUTABILITY (blob), OBJECT_WITHOUT_IMMUTABILITY (buffer), OBJECT_WITHOUT_IMMUTABILITY (face), OBJECT_WITHOUT_IMMUTABILITY (font), + OBJECT_WITH_IMMUTABILITY (blob), OBJECT_WITH_IMMUTABILITY (font_funcs), OBJECT_WITH_IMMUTABILITY (unicode_funcs) };