[gdk-pixbuf] (#91): Have a STORAGE_UNINITIALIZED for construction with all-default properties
- From: Emmanuele Bassi <ebassi src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gdk-pixbuf] (#91): Have a STORAGE_UNINITIALIZED for construction with all-default properties
- Date: Wed, 11 Sep 2019 13:02:58 +0000 (UTC)
commit 3c7740498fd31b6746dd7e04601886766a6644b7
Author: Federico Mena Quintero <federico gnome org>
Date: Wed Dec 12 18:22:30 2018 -0600
(#91): Have a STORAGE_UNINITIALIZED for construction with all-default properties
If one does a plain
GdkPixbuf *pixbuf = g_object_new (GDK_TYPE_PIXBUF, NULL);
Then all the construct properties use their default values. This
means that both "pixels" and "pixel-bytes" get passed as NULL to
gdk_pixbuf_set_property().
Later, trying to get the property for "pixel-bytes" would assert,
incorrectly, because it was trying to create a GBytes from a NULL
pixels storage.
This commit adds a test for that construction case, and tests for
constructing with g_object_new() in general.
Fixes https://gitlab.gnome.org/GNOME/gdk-pixbuf/issues/91
gdk-pixbuf/gdk-pixbuf-private.h | 1 +
gdk-pixbuf/gdk-pixbuf.c | 279 ++++++++++++++++++++++++++--------------
tests/meson.build | 1 +
tests/pixbuf-construction.c | 107 +++++++++++++++
4 files changed, 291 insertions(+), 97 deletions(-)
---
diff --git a/gdk-pixbuf/gdk-pixbuf-private.h b/gdk-pixbuf/gdk-pixbuf-private.h
index 79ccf1531..343724c7e 100644
--- a/gdk-pixbuf/gdk-pixbuf-private.h
+++ b/gdk-pixbuf/gdk-pixbuf-private.h
@@ -50,6 +50,7 @@ typedef struct _GdkPixbufClass GdkPixbufClass;
#define DEFAULT_FILL_COLOR 0x979899ff
typedef enum {
+ STORAGE_UNINITIALIZED,
STORAGE_PIXELS,
STORAGE_BYTES
} Storage;
diff --git a/gdk-pixbuf/gdk-pixbuf.c b/gdk-pixbuf/gdk-pixbuf.c
index 8e39ac416..f38ad8aa3 100644
--- a/gdk-pixbuf/gdk-pixbuf.c
+++ b/gdk-pixbuf/gdk-pixbuf.c
@@ -108,6 +108,7 @@ static void gdk_pixbuf_get_property (GObject *object,
guint prop_id,
GValue *value,
GParamSpec *pspec);
+static void gdk_pixbuf_constructed (GObject *object);
enum
@@ -138,6 +139,7 @@ gdk_pixbuf_init (GdkPixbuf *pixbuf)
pixbuf->n_channels = 3;
pixbuf->bits_per_sample = 8;
pixbuf->has_alpha = FALSE;
+ pixbuf->storage = STORAGE_UNINITIALIZED;
}
static void
@@ -150,6 +152,7 @@ gdk_pixbuf_class_init (GdkPixbufClass *klass)
object_class->finalize = gdk_pixbuf_finalize;
object_class->set_property = gdk_pixbuf_set_property;
object_class->get_property = gdk_pixbuf_get_property;
+ object_class->constructed = gdk_pixbuf_constructed;
#define PIXBUF_PARAM_FLAGS G_PARAM_READWRITE|G_PARAM_CONSTRUCT_ONLY|\
G_PARAM_EXPLICIT_NOTIFY|\
@@ -1239,69 +1242,81 @@ gdk_pixbuf_set_property (GObject *object,
const GValue *value,
GParamSpec *pspec)
{
- GdkPixbuf *pixbuf = GDK_PIXBUF (object);
- gboolean notify = TRUE;
-
- switch (prop_id)
- {
- case PROP_COLORSPACE:
- notify = pixbuf->colorspace != g_value_get_enum (value);
- pixbuf->colorspace = g_value_get_enum (value);
- break;
- case PROP_N_CHANNELS:
- notify = pixbuf->n_channels != g_value_get_int (value);
- pixbuf->n_channels = g_value_get_int (value);
- break;
- case PROP_HAS_ALPHA:
- notify = pixbuf->has_alpha != g_value_get_boolean (value);
- pixbuf->has_alpha = g_value_get_boolean (value);
- break;
- case PROP_BITS_PER_SAMPLE:
- notify = pixbuf->bits_per_sample != g_value_get_int (value);
- pixbuf->bits_per_sample = g_value_get_int (value);
- break;
- case PROP_WIDTH:
- notify = pixbuf->width != g_value_get_int (value);
- pixbuf->width = g_value_get_int (value);
- break;
- case PROP_HEIGHT:
- notify = pixbuf->height != g_value_get_int (value);
- pixbuf->height = g_value_get_int (value);
- break;
- case PROP_ROWSTRIDE:
- notify = pixbuf->rowstride != g_value_get_int (value);
- pixbuf->rowstride = g_value_get_int (value);
- break;
-
- /* The following two are a bit strange. Both PROP_PIXELS and PROP_PIXEL_BYTES are
- * G_PARAM_CONSTRUCT_ONLY properties, which means that GObject will generate default
- * values for any missing one and call us for *both*. So, we need to check whether the
- * passed value is not NULL before actually setting pixbuf->storage.
- */
- case PROP_PIXELS:
- g_assert (pixbuf->s.pixels.pixels == NULL);
- notify = pixbuf->s.pixels.pixels != (guchar *) g_value_get_pointer (value);
- pixbuf->s.pixels.pixels = (guchar *) g_value_get_pointer (value);
- pixbuf->s.pixels.destroy_fn = NULL;
- pixbuf->s.pixels.destroy_fn_data = NULL;
-
- if (pixbuf->s.pixels.pixels != NULL) {
- pixbuf->storage = STORAGE_PIXELS;
- }
- break;
- case PROP_PIXEL_BYTES:
- g_assert (pixbuf->s.bytes.bytes == NULL);
- notify = pixbuf->s.bytes.bytes != g_value_get_boxed (value);
- pixbuf->s.bytes.bytes = g_value_dup_boxed (value);
-
- if (pixbuf->s.bytes.bytes != NULL) {
- pixbuf->storage = STORAGE_BYTES;
- }
- break;
- default:
- G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
- break;
- }
+ GdkPixbuf *pixbuf = GDK_PIXBUF (object);
+ gboolean notify = TRUE;
+
+ switch (prop_id) {
+ case PROP_COLORSPACE:
+ notify = pixbuf->colorspace != g_value_get_enum (value);
+ pixbuf->colorspace = g_value_get_enum (value);
+ break;
+ case PROP_N_CHANNELS:
+ notify = pixbuf->n_channels != g_value_get_int (value);
+ pixbuf->n_channels = g_value_get_int (value);
+ break;
+ case PROP_HAS_ALPHA:
+ notify = pixbuf->has_alpha != g_value_get_boolean (value);
+ pixbuf->has_alpha = g_value_get_boolean (value);
+ break;
+ case PROP_BITS_PER_SAMPLE:
+ notify = pixbuf->bits_per_sample != g_value_get_int (value);
+ pixbuf->bits_per_sample = g_value_get_int (value);
+ break;
+ case PROP_WIDTH:
+ notify = pixbuf->width != g_value_get_int (value);
+ pixbuf->width = g_value_get_int (value);
+ break;
+ case PROP_HEIGHT:
+ notify = pixbuf->height != g_value_get_int (value);
+ pixbuf->height = g_value_get_int (value);
+ break;
+ case PROP_ROWSTRIDE:
+ notify = pixbuf->rowstride != g_value_get_int (value);
+ pixbuf->rowstride = g_value_get_int (value);
+ break;
+
+ /* The following two are a bit strange. Both PROP_PIXELS and
+ * PROP_PIXEL_BYTES are G_PARAM_CONSTRUCT_ONLY properties, which means
+ * that GObject will generate default values for any missing one and
+ * call us for *both*. So, we need to check whether the passed value is
+ * not NULL before actually setting pixbuf->storage.
+ */
+ case PROP_PIXELS: {
+ guchar *pixels = g_value_get_pointer (value);
+
+ if (pixels) {
+ g_assert (pixbuf->storage == STORAGE_UNINITIALIZED);
+
+ pixbuf->storage = STORAGE_PIXELS;
+ pixbuf->s.pixels.pixels = pixels;
+ pixbuf->s.pixels.destroy_fn = NULL;
+ pixbuf->s.pixels.destroy_fn_data = NULL;
+ } else {
+ notify = FALSE;
+ }
+
+ break;
+ }
+
+ case PROP_PIXEL_BYTES: {
+ GBytes *bytes = g_value_get_boxed (value);
+
+ if (bytes) {
+ g_assert (pixbuf->storage == STORAGE_UNINITIALIZED);
+
+ pixbuf->storage = STORAGE_BYTES;
+ pixbuf->s.bytes.bytes = g_value_dup_boxed (value);
+ } else {
+ notify = FALSE;
+ }
+
+ break;
+ }
+
+ default:
+ G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+ break;
+ }
if (notify)
g_object_notify_by_pspec (G_OBJECT (object), pspec);
@@ -1313,39 +1328,109 @@ gdk_pixbuf_get_property (GObject *object,
GValue *value,
GParamSpec *pspec)
{
- GdkPixbuf *pixbuf = GDK_PIXBUF (object);
+ GdkPixbuf *pixbuf = GDK_PIXBUF (object);
- switch (prop_id)
- {
- case PROP_COLORSPACE:
- g_value_set_enum (value, gdk_pixbuf_get_colorspace (pixbuf));
- break;
- case PROP_N_CHANNELS:
- g_value_set_int (value, gdk_pixbuf_get_n_channels (pixbuf));
- break;
- case PROP_HAS_ALPHA:
- g_value_set_boolean (value, gdk_pixbuf_get_has_alpha (pixbuf));
- break;
- case PROP_BITS_PER_SAMPLE:
- g_value_set_int (value, gdk_pixbuf_get_bits_per_sample (pixbuf));
- break;
- case PROP_WIDTH:
- g_value_set_int (value, gdk_pixbuf_get_width (pixbuf));
- break;
- case PROP_HEIGHT:
- g_value_set_int (value, gdk_pixbuf_get_height (pixbuf));
- break;
- case PROP_ROWSTRIDE:
- g_value_set_int (value, gdk_pixbuf_get_rowstride (pixbuf));
- break;
- case PROP_PIXELS:
- g_value_set_pointer (value, gdk_pixbuf_get_pixels (pixbuf));
- break;
- case PROP_PIXEL_BYTES:
- g_value_set_boxed (value, gdk_pixbuf_read_pixel_bytes (pixbuf));
- break;
- default:
- G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
- break;
- }
+ switch (prop_id) {
+ case PROP_COLORSPACE:
+ g_value_set_enum (value, gdk_pixbuf_get_colorspace (pixbuf));
+ break;
+ case PROP_N_CHANNELS:
+ g_value_set_int (value, gdk_pixbuf_get_n_channels (pixbuf));
+ break;
+ case PROP_HAS_ALPHA:
+ g_value_set_boolean (value, gdk_pixbuf_get_has_alpha (pixbuf));
+ break;
+ case PROP_BITS_PER_SAMPLE:
+ g_value_set_int (value, gdk_pixbuf_get_bits_per_sample (pixbuf));
+ break;
+ case PROP_WIDTH:
+ g_value_set_int (value, gdk_pixbuf_get_width (pixbuf));
+ break;
+ case PROP_HEIGHT:
+ g_value_set_int (value, gdk_pixbuf_get_height (pixbuf));
+ break;
+ case PROP_ROWSTRIDE:
+ g_value_set_int (value, gdk_pixbuf_get_rowstride (pixbuf));
+ break;
+ case PROP_PIXELS:
+ g_value_set_pointer (value, gdk_pixbuf_get_pixels (pixbuf));
+ break;
+ case PROP_PIXEL_BYTES:
+ g_value_set_boxed (value, gdk_pixbuf_read_pixel_bytes (pixbuf));
+ break;
+ default:
+ G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+ break;
+ }
+}
+
+static void
+make_storage_invalid (GdkPixbuf *pixbuf)
+{
+ char *buf;
+ gsize bufsize = 3;
+
+ buf = g_new0(char, bufsize);
+
+ pixbuf->storage = STORAGE_BYTES;
+ pixbuf->s.bytes.bytes = g_bytes_new_with_free_func (buf, bufsize, g_free, NULL);
+
+ pixbuf->colorspace = GDK_COLORSPACE_RGB;
+ pixbuf->n_channels = 3;
+ pixbuf->bits_per_sample = 8;
+ pixbuf->width = 1;
+ pixbuf->height = 1;
+ pixbuf->rowstride = 3;
+ pixbuf->has_alpha = FALSE;
+}
+
+static void
+gdk_pixbuf_constructed (GObject *object)
+{
+ GdkPixbuf *pixbuf = GDK_PIXBUF (object);
+
+ G_OBJECT_CLASS (gdk_pixbuf_parent_class)->constructed (object);
+
+ switch (pixbuf->storage) {
+ case STORAGE_UNINITIALIZED:
+ /* This means that neither of the construct properties "pixels" nor "pixel-bytes"
+ * was specified during a call to g_object_new().
+ *
+ * To avoid breaking ABI, we don't emit this warning. We'll want
+ * to emit it once we can have fallible construction.
+ *
+ * g_warning ("pixbuf needs to be constructed with the 'pixels' or 'pixel-bytes'
properties");
+ */
+
+ make_storage_invalid (pixbuf);
+ break;
+
+ case STORAGE_PIXELS:
+ g_assert (pixbuf->s.pixels.pixels != NULL);
+ break;
+
+ case STORAGE_BYTES: {
+ gsize bytes_size;
+ gint width, height;
+ gboolean has_alpha;
+
+ g_assert (pixbuf->s.bytes.bytes != NULL);
+
+ bytes_size = g_bytes_get_size (pixbuf->s.bytes.bytes);
+ width = pixbuf->width;
+ height = pixbuf->height;
+ has_alpha = pixbuf->has_alpha;
+
+ /* This is the same check as in gdk_pixbuf_new_from_bytes() */
+ if (!(bytes_size >= width * height * (has_alpha ? 4 : 3))) {
+ g_error ("GBytes is too small to fit the pixbuf's declared width and height");
+ }
+ break;
+ }
+
+ default:
+ g_assert_not_reached ();
+ }
+
+ g_assert (pixbuf->storage != STORAGE_UNINITIALIZED);
}
diff --git a/tests/meson.build b/tests/meson.build
index aaa1817e7..5116fc2b9 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -47,6 +47,7 @@ resources_h = custom_target('resources.h',
# - io: Loading/saving
# - ops: Pixel operations
installed_tests = [
+ [ 'pixbuf-construction', ['conform'], ],
[ 'animation', ['format'], ],
[ 'cve-2015-4491', ['security'], true ],
[ 'pixbuf-fail', ['conform', 'slow'], ],
diff --git a/tests/pixbuf-construction.c b/tests/pixbuf-construction.c
new file mode 100644
index 000000000..5e9e22b16
--- /dev/null
+++ b/tests/pixbuf-construction.c
@@ -0,0 +1,107 @@
+/* -*- Mode: C; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* GdkPixbuf library - tests for GdkPixbuf constructors
+ *
+ * Copyright (C) 2018 Federico Mena Quintero
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "config.h"
+#include "gdk-pixbuf/gdk-pixbuf.h"
+#include "test-common.h"
+
+static void
+test_no_construct_properties (void)
+{
+ GdkPixbuf *pixbuf = g_object_new (GDK_TYPE_PIXBUF, NULL);
+ GBytes *bytes;
+ guchar *pixels;
+
+ g_assert_cmpint (gdk_pixbuf_get_width (pixbuf), ==, 1);
+ g_assert_cmpint (gdk_pixbuf_get_height (pixbuf), ==, 1);
+
+ g_object_get (pixbuf, "pixel-bytes", &bytes, NULL);
+ g_assert (bytes != NULL);
+ g_bytes_unref (bytes);
+
+ g_object_get (pixbuf, "pixels", &pixels, NULL);
+ g_assert (pixels != NULL);
+}
+
+#define WIDTH 10
+#define HEIGHT 20
+#define BUFFER_SIZE (WIDTH * HEIGHT * 4)
+#define ROWSTRIDE (WIDTH * 4)
+
+static void
+test_pixels (void)
+{
+ guchar *pixels = g_new0 (guchar, BUFFER_SIZE);
+
+ GdkPixbuf *pixbuf = g_object_new (GDK_TYPE_PIXBUF,
+ "width", WIDTH,
+ "height", HEIGHT,
+ "rowstride", ROWSTRIDE,
+ "bits-per-sample", 8,
+ "n-channels", 3,
+ "has-alpha", TRUE,
+ "pixels", pixels,
+ NULL);
+
+ g_assert (gdk_pixbuf_get_pixels (pixbuf) == pixels);
+ g_assert_cmpint (gdk_pixbuf_get_width (pixbuf), ==, WIDTH);
+ g_assert_cmpint (gdk_pixbuf_get_height (pixbuf), ==, HEIGHT);
+ g_assert_cmpint (gdk_pixbuf_get_rowstride (pixbuf), ==, ROWSTRIDE);
+
+ g_object_unref (pixbuf);
+ g_free (pixels);
+}
+
+static void
+test_bytes (void)
+{
+ guchar *pixels = g_new0 (guchar, BUFFER_SIZE);
+ GBytes *bytes = g_bytes_new_take (pixels, BUFFER_SIZE);
+ GdkPixbuf *pixbuf = g_object_new (GDK_TYPE_PIXBUF,
+ "width", WIDTH,
+ "height", HEIGHT,
+ "rowstride", ROWSTRIDE,
+ "bits-per-sample", 8,
+ "n-channels", 3,
+ "has-alpha", TRUE,
+ "pixel-bytes", bytes,
+ NULL);
+
+ GBytes *read_bytes = gdk_pixbuf_read_pixel_bytes (pixbuf);
+
+ g_assert (read_bytes == bytes);
+ g_assert_cmpint (gdk_pixbuf_get_width (pixbuf), ==, WIDTH);
+ g_assert_cmpint (gdk_pixbuf_get_height (pixbuf), ==, HEIGHT);
+ g_assert_cmpint (gdk_pixbuf_get_rowstride (pixbuf), ==, ROWSTRIDE);
+
+ g_bytes_unref (read_bytes);
+ g_object_unref (pixbuf);
+ g_bytes_unref (bytes);
+}
+
+int
+main (int argc, char **argv)
+{
+ g_test_init (&argc, &argv, NULL);
+
+ g_test_add_func ("/pixbuf/construction/no_construct_properties", test_no_construct_properties);
+ g_test_add_func ("/pixbuf/construction/pixels", test_pixels);
+ g_test_add_func ("/pixbuf/construction/bytes", test_bytes);
+
+ return g_test_run ();
+}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
