[gdk-pixbuf/static-analysis] png: Clean up PNG saving
- From: Emmanuele Bassi <ebassi src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gdk-pixbuf/static-analysis] png: Clean up PNG saving
- Date: Tue, 10 Nov 2020 01:21:08 +0000 (UTC)
commit a0ff5afa5147f9f89a6f8d1764bf4e4e15676337
Author: Emmanuele Bassi <ebassi gnome org>
Date: Tue Nov 10 01:19:14 2020 +0000
png: Clean up PNG saving
Iterate over the key/value arrays once, using GArray to store the text
chunks after validating the keys.
gdk-pixbuf/io-png.c | 534 ++++++++++++++++++++++++++--------------------------
1 file changed, 263 insertions(+), 271 deletions(-)
---
diff --git a/gdk-pixbuf/io-png.c b/gdk-pixbuf/io-png.c
index 3778f3469b..26130fddbc 100644
--- a/gdk-pixbuf/io-png.c
+++ b/gdk-pixbuf/io-png.c
@@ -885,300 +885,288 @@ png_save_to_callback_flush_func (png_structp png_ptr)
;
}
-static gboolean real_save_png (GdkPixbuf *pixbuf,
- gchar **keys,
- gchar **values,
- GError **error,
- gboolean to_callback,
- FILE *f,
- GdkPixbufSaveFunc save_func,
- gpointer user_data)
+static gboolean
+real_save_png (GdkPixbuf *pixbuf,
+ int n_keys,
+ gchar **keys,
+ gchar **values,
+ GError **error,
+ gboolean to_callback,
+ FILE *f,
+ GdkPixbufSaveFunc save_func,
+ gpointer user_data)
{
- png_structp png_ptr = NULL;
- png_infop info_ptr;
- png_textp text_ptr = NULL;
- guchar *ptr;
- guchar *pixels;
- int y;
- int i;
- png_bytep row_ptr;
- png_color_8 sig_bit;
- int w, h, rowstride;
- int has_alpha;
- int bpc;
- int num_keys;
- int compression = -1;
- int x_density = 0;
- int y_density = 0;
- gboolean success = TRUE;
- guchar *icc_profile = NULL;
- gsize icc_profile_size = 0;
- SaveToFunctionIoPtr to_callback_ioptr;
-
- num_keys = 0;
-
- if (keys && *keys) {
- gchar **kiter = keys;
- gchar **viter = values;
-
- while (*kiter) {
- if (strncmp (*kiter, "tEXt::", 6) == 0) {
- gchar *key = *kiter + 6;
- int len = strlen (key);
- if (len < 1 || len > 79) {
- g_set_error_literal (error,
- GDK_PIXBUF_ERROR,
- GDK_PIXBUF_ERROR_BAD_OPTION,
- _("Keys for PNG text chunks must have at least 1
and at most 79 characters."));
- success = FALSE;
- goto cleanup;
- }
- for (i = 0; i < len; i++) {
- if ((guchar) key[i] > 127) {
- g_set_error_literal (error,
- GDK_PIXBUF_ERROR,
- GDK_PIXBUF_ERROR_BAD_OPTION,
- _("Keys for PNG text chunks must be
ASCII characters."));
- success = FALSE;
- goto cleanup;
- }
- }
- num_keys++;
- } else if (strcmp (*kiter, "icc-profile") == 0) {
- /* decode from base64 */
- icc_profile = g_base64_decode (*viter, &icc_profile_size);
- if (icc_profile_size < 127) {
- /* This is a user-visible error */
- g_set_error (error,
- GDK_PIXBUF_ERROR,
- GDK_PIXBUF_ERROR_BAD_OPTION,
- _("Color profile has invalid length %d."),
- (gint)icc_profile_size);
- success = FALSE;
- goto cleanup;
- }
- } else if (strcmp (*kiter, "compression") == 0) {
- char *endptr = NULL;
- compression = strtol (*viter, &endptr, 10);
-
- if (endptr == *viter) {
- g_set_error (error,
- GDK_PIXBUF_ERROR,
- GDK_PIXBUF_ERROR_BAD_OPTION,
- _("PNG compression level must be a value between 0 and
9; value “%s” could not be parsed."),
- *viter);
- success = FALSE;
- goto cleanup;
- }
- if (compression < 0 || compression > 9) {
- /* This is a user-visible error;
- * lets people skip the range-checking
- * in their app.
- */
- g_set_error (error,
- GDK_PIXBUF_ERROR,
- GDK_PIXBUF_ERROR_BAD_OPTION,
- _("PNG compression level must be a value between 0 and
9; value “%d” is not allowed."),
- compression);
- success = FALSE;
- goto cleanup;
- }
- } else if (strcmp (*kiter, "x-dpi") == 0) {
- char *endptr = NULL;
- x_density = strtol (*viter, &endptr, 10);
- if (endptr == *viter)
- x_density = -1;
-
- if (x_density <= 0) {
- /* This is a user-visible error;
- * lets people skip the range-checking
- * in their app.
- */
- g_set_error (error,
- GDK_PIXBUF_ERROR,
- GDK_PIXBUF_ERROR_BAD_OPTION,
- _("PNG x-dpi must be greater than zero; value “%s” is
not allowed."),
- *viter);
-
- success = FALSE;
- goto cleanup;
- }
- } else if (strcmp (*kiter, "y-dpi") == 0) {
- char *endptr = NULL;
- y_density = strtol (*viter, &endptr, 10);
- if (endptr == *viter)
- y_density = -1;
-
- if (y_density <= 0) {
- /* This is a user-visible error;
- * lets people skip the range-checking
- * in their app.
- */
- g_set_error (error,
- GDK_PIXBUF_ERROR,
- GDK_PIXBUF_ERROR_BAD_OPTION,
- _("PNG y-dpi must be greater than zero; value “%s” is
not allowed."),
- *viter);
-
- success = FALSE;
- goto cleanup;
- }
- } else {
- g_warning ("Unrecognized parameter (%s) passed to PNG saver.", *kiter);
- }
-
- ++kiter;
- ++viter;
- }
- }
-
- if (num_keys > 0) {
- gchar **kiter = keys;
- gchar **viter = values;
-
- text_ptr = g_new0 (png_text, num_keys);
- for (i = 0; i < num_keys; i++) {
- if (strncmp (*kiter, "tEXt::", 6) != 0) {
- kiter++;
- viter++;
- }
-
- text_ptr[i].compression = PNG_TEXT_COMPRESSION_NONE;
- text_ptr[i].key = *kiter + 6;
- text_ptr[i].text = g_convert (*viter, -1,
- "ISO-8859-1", "UTF-8",
- NULL, &text_ptr[i].text_length,
- NULL);
+ png_structp png_ptr = NULL;
+ png_infop info_ptr;
+ guchar *ptr;
+ guchar *pixels;
+ int y;
+ png_bytep row_ptr;
+ png_color_8 sig_bit;
+ int w, h, rowstride;
+ int has_alpha;
+ int bpc;
+ int compression = -1;
+ int x_density = 0;
+ int y_density = 0;
+ gboolean success = TRUE;
+ guchar *icc_profile = NULL;
+ gsize icc_profile_size = 0;
+ SaveToFunctionIoPtr to_callback_ioptr;
+ int num_keys = 0;
+ png_textp text_ptr = NULL;
+ GArray *text_data = NULL;
+
+ text_data = g_array_sized_new (FALSE, TRUE, sizeof (png_text), n_keys);
+
+ for (int i = 0; i < n_keys; i++) {
+ const char *key = keys[i];
+ const char *value = values[i];
+
+ if (strncmp (key, "tEXt::", 6) == 0) {
+ const char *unprefixed_key = key + 6;
+ int len = strlen (unprefixed_key);
+ png_text text;
+
+ if (len < 1 || len > 79) {
+ /* Translators notice: '%s' is the name of the
+ * PNG text key
+ */
+ g_set_error (error, GDK_PIXBUF_ERROR,
+ GDK_PIXBUF_ERROR_BAD_OPTION,
+ _("Invalid key “%s”. Keys for PNG text chunks must have at
least 1 and at most 79 characters."),
+ unprefixed_key);
+ success = FALSE;
+ goto cleanup;
+ }
+
+ for (int i = 0; i < len; i++) {
+ if ((guchar) unprefixed_key[i] > 127) {
+ /* Translators notice: '%s' is the name of
+ * the PNG text key
+ */
+ g_set_error (error, GDK_PIXBUF_ERROR,
+ GDK_PIXBUF_ERROR_BAD_OPTION,
+ _("Invalid key “%s”. Keys for PNG text chunks must be
ASCII characters."),
+ unprefixed_key);
+ success = FALSE;
+ goto cleanup;
+ }
+ }
+
+ text.compression = PNG_TEXT_COMPRESSION_NONE;
+ text.key = unprefixed_key;
+ text.text = g_convert (value, -1,
+ "ISO-8859-1", "UTF-8",
+ NULL,
+ &text.text_length,
+ NULL);
#ifdef PNG_iTXt_SUPPORTED
- if (!text_ptr[i].text) {
- text_ptr[i].compression = PNG_ITXT_COMPRESSION_NONE;
- text_ptr[i].text = g_strdup (*viter);
- text_ptr[i].text_length = 0;
- text_ptr[i].itxt_length = strlen (text_ptr[i].text);
- text_ptr[i].lang = NULL;
- text_ptr[i].lang_key = NULL;
- }
+ if (text.text == NULL) {
+ text.compression = PNG_ITXT_COMPRESSION_NONE;
+ text.text = g_strdup (value);
+ text.text_length = 0;
+ text.itxt_length = strlen (value);
+ text.lang = NULL;
+ text.lang_key = NULL;
+ }
#endif
- if (!text_ptr[i].text) {
- gint j;
- g_set_error (error,
+ if (text.text == NULL) {
+ g_set_error (error,
GDK_PIXBUF_ERROR,
GDK_PIXBUF_ERROR_BAD_OPTION,
- _("Value for PNG text chunk %s cannot be converted to ISO-8859-1
encoding."), *kiter + 6);
- for (j = 0; j < i; j++)
- g_free (text_ptr[j].text);
- g_free (text_ptr);
- return FALSE;
- }
-
- kiter++;
- viter++;
- }
- }
-
- bpc = gdk_pixbuf_get_bits_per_sample (pixbuf);
- w = gdk_pixbuf_get_width (pixbuf);
- h = gdk_pixbuf_get_height (pixbuf);
- rowstride = gdk_pixbuf_get_rowstride (pixbuf);
- has_alpha = gdk_pixbuf_get_has_alpha (pixbuf);
- pixels = gdk_pixbuf_get_pixels (pixbuf);
-
- /* Guaranteed by the caller. */
- g_assert (w >= 0);
- g_assert (h >= 0);
- g_assert (rowstride >= 0);
-
- png_ptr = png_create_write_struct (PNG_LIBPNG_VER_STRING,
- error,
- png_simple_error_callback,
- png_simple_warning_callback);
- if (png_ptr == NULL) {
- success = FALSE;
- goto cleanup;
- }
-
- info_ptr = png_create_info_struct (png_ptr);
- if (info_ptr == NULL) {
- success = FALSE;
- goto cleanup;
- }
- if (setjmp (png_jmpbuf(png_ptr))) {
- success = FALSE;
- goto cleanup;
- }
-
- if (num_keys > 0) {
- png_set_text (png_ptr, info_ptr, text_ptr, num_keys);
- }
-
- if (to_callback) {
- to_callback_ioptr.save_func = save_func;
- to_callback_ioptr.user_data = user_data;
- to_callback_ioptr.error = error;
- png_set_write_fn (png_ptr, &to_callback_ioptr,
- png_save_to_callback_write_func,
- png_save_to_callback_flush_func);
- } else {
- png_init_io (png_ptr, f);
- }
-
- if (compression >= 0)
- png_set_compression_level (png_ptr, compression);
+ _("Value for PNG text chunk '%s' cannot be converted to
ISO-8859-1 encoding."), unprefixed_key);
+ success = FALSE;
+ goto cleanup;
+ }
+
+ g_array_append_val (text_data, text);
+ } else if (strcmp (key, "icc-profile") == 0) {
+ icc_profile = g_base64_decode (value, &icc_profile_size);
+
+ if (icc_profile_size < 127) {
+ g_set_error (error, GDK_PIXBUF_ERROR,
+ GDK_PIXBUF_ERROR_BAD_OPTION,
+ _("Color profile has invalid length %d"),
+ (int) icc_profile_size);
+ success = FALSE;
+ goto cleanup;
+ }
+ } else if (strcmp (key, "compression") == 0) {
+ char *endptr = NULL;
+
+ compression = strtol (value, &endptr, 10);
+ if (endptr == value || (compression < 0 || compression > 9)) {
+ g_set_error (error, GDK_PIXBUF_ERROR,
+ GDK_PIXBUF_ERROR_BAD_OPTION,
+ _("PNG compression level must be a value between 0 and 9; value
“%s” is invalid"),
+ value);
+ success = FALSE;
+ goto cleanup;
+ }
+ } else if (strcmp (key, "x-dpi") == 0 || strcmp (key, "y-dpi") == 0) {
+ gboolean is_horizontal = strcmp (key, "x-dpi") == 0;
+ char *endptr = NULL;
+
+ int dpi = strtol (value, &endptr, 10);
+
+ if (endptr == value || dpi <= 0) {
+ g_set_error (error, GDK_PIXBUF_ERROR,
+ GDK_PIXBUF_ERROR_BAD_OPTION,
+ _("PNG %s must be greater than zero; value “%s” is not
allowed"),
+ is_horizontal ? "x-dpi" : "y-dpi",
+ value);
+ success = FALSE;
+ goto cleanup;
+ }
+
+ if (is_horizontal) {
+ x_density = dpi;
+ } else {
+ y_density = dpi;
+ }
+ } else {
+ g_warning ("Unrecognized parameter “%s” passed to the PNG saver", key);
+ }
+ }
+
+ bpc = gdk_pixbuf_get_bits_per_sample (pixbuf);
+ w = gdk_pixbuf_get_width (pixbuf);
+ h = gdk_pixbuf_get_height (pixbuf);
+ rowstride = gdk_pixbuf_get_rowstride (pixbuf);
+ has_alpha = gdk_pixbuf_get_has_alpha (pixbuf);
+ pixels = gdk_pixbuf_get_pixels (pixbuf);
+
+ if (text_data->len > 0) {
+ num_keys = text_data->len;
+ text_ptr = (png_textp) g_array_free (text_data, FALSE);
+ text_data = NULL;
+ } else {
+ g_clear_pointer (&text_data, g_array_unref);
+ num_keys = 0;
+ text_ptr = NULL;
+ }
+
+ /* Guaranteed by the caller. */
+ g_assert (w >= 0);
+ g_assert (h >= 0);
+ g_assert (rowstride >= 0);
+
+ png_ptr = png_create_write_struct (PNG_LIBPNG_VER_STRING,
+ error,
+ png_simple_error_callback,
+ png_simple_warning_callback);
+ if (png_ptr == NULL) {
+ success = FALSE;
+ goto cleanup;
+ }
+
+ info_ptr = png_create_info_struct (png_ptr);
+ if (info_ptr == NULL) {
+ success = FALSE;
+ goto cleanup;
+ }
+
+ if (setjmp (png_jmpbuf (png_ptr))) {
+ success = FALSE;
+ goto cleanup;
+ }
+
+ if (num_keys > 0) {
+ png_set_text (png_ptr, info_ptr, text_ptr, num_keys);
+ }
+
+ if (to_callback) {
+ to_callback_ioptr.save_func = save_func;
+ to_callback_ioptr.user_data = user_data;
+ to_callback_ioptr.error = error;
+ png_set_write_fn (png_ptr, &to_callback_ioptr,
+ png_save_to_callback_write_func,
+ png_save_to_callback_flush_func);
+ } else {
+ png_init_io (png_ptr, f);
+ }
+
+ if (compression >= 0) {
+ png_set_compression_level (png_ptr, compression);
+ }
#ifdef PNG_pHYs_SUPPORTED
- if (x_density > 0 && y_density > 0)
- png_set_pHYs (png_ptr, info_ptr, DPI_TO_DPM (x_density), DPI_TO_DPM (y_density),
PNG_RESOLUTION_METER);
+ if (x_density > 0 && y_density > 0) {
+ png_set_pHYs (png_ptr, info_ptr,
+ DPI_TO_DPM (x_density),
+ DPI_TO_DPM (y_density),
+ PNG_RESOLUTION_METER);
+ }
#endif
#if defined(PNG_iCCP_SUPPORTED)
/* the proper ICC profile title is encoded in the profile */
if (icc_profile != NULL) {
png_set_iCCP (png_ptr, info_ptr,
- "ICC profile", PNG_COMPRESSION_TYPE_BASE,
- (png_bytep) icc_profile, icc_profile_size);
+ "ICC profile",
+ PNG_COMPRESSION_TYPE_BASE,
+ (png_bytep) icc_profile,
+ icc_profile_size);
}
#endif
- if (has_alpha) {
- png_set_IHDR (png_ptr, info_ptr, w, h, bpc,
- PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE,
- PNG_COMPRESSION_TYPE_BASE, PNG_FILTER_TYPE_BASE);
- } else {
- png_set_IHDR (png_ptr, info_ptr, w, h, bpc,
- PNG_COLOR_TYPE_RGB, PNG_INTERLACE_NONE,
- PNG_COMPRESSION_TYPE_BASE, PNG_FILTER_TYPE_BASE);
- }
- /* Note bpc is always 8 */
- sig_bit.red = bpc;
- sig_bit.green = bpc;
- sig_bit.blue = bpc;
- sig_bit.alpha = bpc;
- png_set_sBIT (png_ptr, info_ptr, &sig_bit);
- png_write_info (png_ptr, info_ptr);
- png_set_packing (png_ptr);
-
- for (y = 0, ptr = pixels; y < h; y++, ptr += rowstride) {
- row_ptr = (png_bytep)ptr;
- png_write_rows (png_ptr, &row_ptr, 1);
- }
-
- png_write_end (png_ptr, info_ptr);
+ if (has_alpha) {
+ png_set_IHDR (png_ptr, info_ptr, w, h, bpc,
+ PNG_COLOR_TYPE_RGB_ALPHA,
+ PNG_INTERLACE_NONE,
+ PNG_COMPRESSION_TYPE_BASE,
+ PNG_FILTER_TYPE_BASE);
+ } else {
+ png_set_IHDR (png_ptr, info_ptr, w, h, bpc,
+ PNG_COLOR_TYPE_RGB,
+ PNG_INTERLACE_NONE,
+ PNG_COMPRESSION_TYPE_BASE,
+ PNG_FILTER_TYPE_BASE);
+ }
+
+ /* Note bpc is always 8 */
+ sig_bit.red = bpc;
+ sig_bit.green = bpc;
+ sig_bit.blue = bpc;
+ sig_bit.alpha = bpc;
+ png_set_sBIT (png_ptr, info_ptr, &sig_bit);
+ png_write_info (png_ptr, info_ptr);
+ png_set_packing (png_ptr);
+
+ for (y = 0, ptr = pixels; y < h; y++, ptr += rowstride) {
+ row_ptr = (png_bytep)ptr;
+ png_write_rows (png_ptr, &row_ptr, 1);
+ }
+
+ png_write_end (png_ptr, info_ptr);
+
+ for (int i = 0; i < num_keys; i++) {
+ g_free (text_ptr[i].text);
+ }
+
+ g_free (text_ptr);
cleanup:
- if (png_ptr != NULL)
+ if (png_ptr != NULL) {
png_destroy_write_struct (&png_ptr, &info_ptr);
+ }
- g_free (icc_profile);
+ if (text_data != NULL) {
+ for (guint i = 0; i < text_data->len; i++) {
+ png_textp text = &g_array_index (text_data, png_text, i);
- if (text_ptr != NULL) {
- for (i = 0; i < num_keys; i++)
- g_free (text_ptr[i].text);
- g_free (text_ptr);
+ g_free (text->text);
+ }
+
+ g_array_unref (text_data);
}
- return success;
+ g_free (icc_profile);
+
+ return success;
}
static gboolean
@@ -1188,7 +1176,9 @@ gdk_pixbuf__png_image_save (FILE *f,
gchar **values,
GError **error)
{
- return real_save_png (pixbuf, keys, values, error,
+ int n_keys = keys != NULL ? g_strv_length (keys) : 0;
+
+ return real_save_png (pixbuf, n_keys, keys, values, error,
FALSE, f, NULL, NULL);
}
@@ -1200,7 +1190,9 @@ gdk_pixbuf__png_image_save_to_callback (GdkPixbufSaveFunc save_func,
gchar **values,
GError **error)
{
- return real_save_png (pixbuf, keys, values, error,
+ int n_keys = keys != NULL ? g_strv_length (keys) : 0;
+
+ return real_save_png (pixbuf, n_keys, keys, values, error,
TRUE, NULL, save_func, user_data);
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
