From eb0bf3ae6688b7e98a706df2ad2714c071d77e22 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 6 Aug 2014 15:36:41 -0400 Subject: [PATCH] Relax inert checks Previously, when creating an object from inert inputs (eg: "hb_font_create(hb_face_get_empty())") we returned the inert empty object. This is not helpful as there are legitimate usecases to do that. We now never return the inert object unless allocation failed. Tests are revised to reflect. --- src/hb-face.cc | 4 ++-- src/hb-font.cc | 4 +--- src/hb-shape-plan.cc | 7 +++++-- test/api/test-font.c | 10 +++++----- test/api/test-object.c | 34 +++++++++++++++++----------------- 5 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/hb-face.cc b/src/hb-face.cc index 9348af7bf..f38f04734 100644 --- a/src/hb-face.cc +++ b/src/hb-face.cc @@ -165,8 +165,8 @@ hb_face_create (hb_blob_t *blob, { hb_face_t *face; - if (unlikely (!blob || !hb_blob_get_length (blob))) - return hb_face_get_empty (); + if (unlikely (!blob)) + blob = hb_blob_get_empty (); hb_face_for_data_closure_t *closure = _hb_face_for_data_closure_create (OT::Sanitizer::sanitize (hb_blob_reference (blob)), index); diff --git a/src/hb-font.cc b/src/hb-font.cc index d42db5985..3df41bd38 100644 --- a/src/hb-font.cc +++ b/src/hb-font.cc @@ -854,8 +854,6 @@ hb_font_create (hb_face_t *face) if (unlikely (!face)) face = hb_face_get_empty (); - if (unlikely (hb_object_is_inert (face))) - return hb_font_get_empty (); if (!(font = hb_object_create ())) return hb_font_get_empty (); @@ -880,7 +878,7 @@ hb_font_t * hb_font_create_sub_font (hb_font_t *parent) { if (unlikely (!parent)) - return hb_font_get_empty (); + parent = hb_font_get_empty (); hb_font_t *font = hb_font_create (parent->face); diff --git a/src/hb-shape-plan.cc b/src/hb-shape-plan.cc index 2166173f9..4ccf90ef1 100644 --- a/src/hb-shape-plan.cc +++ b/src/hb-shape-plan.cc @@ -126,7 +126,7 @@ hb_shape_plan_create (hb_face_t *face, if (unlikely (!face)) face = hb_face_get_empty (); - if (unlikely (!props || hb_object_is_inert (face))) + if (unlikely (!props)) return hb_shape_plan_get_empty (); if (num_user_features && !(features = (hb_feature_t *) malloc (num_user_features * sizeof (hb_feature_t)))) return hb_shape_plan_get_empty (); @@ -294,7 +294,6 @@ hb_shape_plan_execute (hb_shape_plan_t *shape_plan, shape_plan->shaper_func); if (unlikely (hb_object_is_inert (shape_plan) || - hb_object_is_inert (font) || hb_object_is_inert (buffer))) return false; @@ -453,6 +452,10 @@ retry: hb_shape_plan_t *shape_plan = hb_shape_plan_create (face, props, user_features, num_user_features, shaper_list); + /* Don't add to the cache if face is inert. */ + if (unlikely (hb_object_is_inert (face))) + return shape_plan; + /* Don't add the plan to the cache if there were user features with non-global ranges */ if (hb_non_global_user_features_present (user_features, num_user_features)) diff --git a/test/api/test-font.c b/test/api/test-font.c index 6b6a5039f..2fc06313b 100644 --- a/test/api/test-font.c +++ b/test/api/test-font.c @@ -36,8 +36,8 @@ static void test_face_empty (void) { g_assert (hb_face_get_empty ()); - g_assert (hb_face_get_empty () == hb_face_create (hb_blob_get_empty (), 0)); - g_assert (hb_face_get_empty () == hb_face_create (NULL, 0)); + g_assert (hb_face_get_empty () != hb_face_create (hb_blob_get_empty (), 0)); + g_assert (hb_face_get_empty () != hb_face_create (NULL, 0)); g_assert (hb_face_reference_table (hb_face_get_empty (), HB_TAG ('h','e','a','d')) == hb_blob_get_empty ()); @@ -348,9 +348,9 @@ static void test_font_empty (void) { g_assert (hb_font_get_empty ()); - g_assert (hb_font_get_empty () == hb_font_create (hb_face_get_empty ())); - g_assert (hb_font_get_empty () == hb_font_create (NULL)); - g_assert (hb_font_get_empty () == hb_font_create_sub_font (NULL)); + g_assert (hb_font_get_empty () != hb_font_create (hb_face_get_empty ())); + g_assert (hb_font_get_empty () != hb_font_create (NULL)); + g_assert (hb_font_get_empty () != hb_font_create_sub_font (NULL)); g_assert (hb_font_is_immutable (hb_font_get_empty ())); g_assert (hb_font_get_face (hb_font_get_empty ()) == hb_face_get_empty ()); diff --git a/test/api/test-object.c b/test/api/test-object.c index 3afe6ae87..4ea6f7cbf 100644 --- a/test/api/test-object.c +++ b/test/api/test-object.c @@ -36,7 +36,7 @@ create_blob (void) return hb_blob_create (data, sizeof (data), HB_MEMORY_MODE_READONLY, NULL, NULL); } static void * -create_blob_inert (void) +create_blob_from_inert (void) { return hb_blob_create (NULL, 0, HB_MEMORY_MODE_DUPLICATE, NULL, NULL); } @@ -47,7 +47,7 @@ create_buffer (void) return hb_buffer_create (); } static void * -create_buffer_inert (void) +create_buffer_from_inert (void) { return NULL; } @@ -58,7 +58,7 @@ create_set (void) return hb_set_create (); } static void * -create_set_inert (void) +create_set_from_inert (void) { return NULL; } @@ -72,7 +72,7 @@ create_face (void) return face; } static void * -create_face_inert (void) +create_face_from_inert (void) { return hb_face_create (hb_blob_get_empty (), 0); } @@ -86,7 +86,7 @@ create_font (void) return font; } static void * -create_font_inert (void) +create_font_from_inert (void) { return hb_font_create (hb_face_get_empty ()); } @@ -97,7 +97,7 @@ create_font_funcs (void) return hb_font_funcs_create (); } static void * -create_font_funcs_inert (void) +create_font_funcs_from_inert (void) { return NULL; } @@ -108,9 +108,9 @@ create_unicode_funcs (void) return hb_unicode_funcs_create (NULL); } static void * -create_unicode_funcs_inert (void) +create_unicode_funcs_from_inert (void) { - return hb_unicode_funcs_get_default (); + return hb_unicode_funcs_create (hb_unicode_funcs_get_empty ()); } @@ -125,7 +125,7 @@ typedef hb_bool_t (*is_immutable_func_t) (void *obj); typedef struct { create_func_t create; - create_func_t create_inert; + create_func_t create_from_inert; create_func_t get_empty; reference_func_t reference; destroy_func_t destroy; @@ -139,7 +139,7 @@ typedef struct { #define OBJECT_WITHOUT_IMMUTABILITY(name) \ { \ (create_func_t) create_##name, \ - (create_func_t) create_##name##_inert, \ + (create_func_t) create_##name##_from_inert, \ (create_func_t) hb_##name##_get_empty, \ (reference_func_t) hb_##name##_reference, \ (destroy_func_t) hb_##name##_destroy, \ @@ -152,7 +152,7 @@ typedef struct { #define OBJECT_WITH_IMMUTABILITY(name) \ { \ (create_func_t) create_##name, \ - (create_func_t) create_##name##_inert, \ + (create_func_t) create_##name##_from_inert, \ (create_func_t) hb_##name##_get_empty, \ (reference_func_t) hb_##name##_reference, \ (destroy_func_t) hb_##name##_destroy, \ @@ -340,8 +340,8 @@ test_object (void) { data_t data[2] = {{MAGIC0, FALSE}, {MAGIC1, FALSE}}; - g_test_message ("->create_inert()"); - obj = o->create_inert (); + g_test_message ("->create_from_inert()"); + obj = o->create_from_inert (); if (!obj) continue; if (obj == o->get_empty ()) @@ -351,10 +351,10 @@ test_object (void) o->destroy (obj); if (o->is_immutable) - g_assert (o->is_immutable (obj)); + g_assert (!o->is_immutable (obj)); - g_assert (!o->set_user_data (obj, &key[0], &data[0], free_up0, TRUE)); - g_assert (!o->get_user_data (obj, &key[0])); + g_assert (o->set_user_data (obj, &key[0], &data[0], free_up0, TRUE)); + g_assert (o->get_user_data (obj, &key[0])); o->destroy (obj); o->destroy (obj); @@ -362,7 +362,7 @@ test_object (void) o->destroy (obj); o->destroy (obj); - g_assert (!data[0].freed); + g_assert (data[0].freed); } } }