[libpng16Cleanup of png_set_filler(). This function does very different things

on read and write.  In libpng 1.6 the two cases can be distinguished and
considerable code cleanup, and extra error checking, is possible.  This
makes calls on the write side that have no effect be ignored with a
png_app_error(), which can be disabled in the app using
png_set_benign_errors(), and removes the spurious use of usr_channels
on the read side.

ANNOUNCE

@@ -415,6 +415,13 @@ Version 1.6.0beta27 [August 10, 2012]
     it with strncpy().
   Eliminated use of png_sizeof(); use sizeof() instead.
   Use a consistent style for (sizeof type) and (sizeof (array))
+  Cleanup of png_set_filler().  This function does very different things on
+    read and write.  In libpng 1.6 the two cases can be distinguished and
+    considerable code cleanup, and extra error checking, is possible.  This
+    makes calls on the write side that have no effect be ignored with a
+    png_app_error(), which can be disabled in the app using
+    png_set_benign_errors(), and removes the spurious use of usr_channels
+    on the read side.
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

CHANGES

@@ -4166,6 +4166,13 @@ Version 1.6.0beta27 [August 10, 2012]
     it with strncpy().
   Eliminated use of png_sizeof(); use sizeof() instead.
   Use a consistent style for (sizeof type) and (sizeof (array))
+  Cleanup of png_set_filler().  This function does very different things on
+    read and write.  In libpng 1.6 the two cases can be distinguished and
+    considerable code cleanup, and extra error checking, is possible.  This
+    makes calls on the write side that have no effect be ignored with a
+    png_app_error(), which can be disabled in the app using
+    png_set_benign_errors(), and removes the spurious use of usr_channels
+    on the read side.
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

pngpriv.h

@@ -1577,7 +1577,7 @@ PNG_INTERNAL_FUNCTION(void,png_formatted_warning,(png_const_structrp png_ptr,
 PNG_INTERNAL_FUNCTION(void,png_app_warning,(png_const_structrp png_ptr,
    png_const_charp message),PNG_EMPTY);
    /* The application provided invalid parameters to an API function or called
-    * an API function at the wrong time, libpng can completely recovered.
+    * an API function at the wrong time, libpng can completely recover.
     */
 
 PNG_INTERNAL_FUNCTION(void,png_app_error,(png_const_structrp png_ptr,

pngstruct.h

@@ -252,7 +252,9 @@ struct png_struct_def
    png_byte usr_bit_depth;    /* bit depth of users row: write only */
    png_byte pixel_depth;      /* number of bits per pixel */
    png_byte channels;         /* number of channels in file */
+#ifdef PNG_WRITE_SUPPORTED
    png_byte usr_channels;     /* channels at start of write: write only */
+#endif
    png_byte sig_bytes;        /* magic bytes read/written from start of file */
    png_byte maximum_pixel_depth;
                               /* pixel depth used for the row buffers */

pngtrans.c

@@ -122,32 +122,79 @@ png_set_filler(png_structrp png_ptr, png_uint_32 filler, int filler_loc)
    if (png_ptr == NULL)
       return;
 
+   /* In libpng 1.6 it is possible to determine whether this is a read or write
+    * operation and therefore to do more checking here for a valid call.
+    */
+   if (png_ptr->mode & PNG_IS_READ_STRUCT)
+   {
+#     ifdef PNG_READ_FILLER_SUPPORTED
+         /* On read png_set_filler is always valid, regardless of the base PNG
+          * format, because other transformations can give a format where the
+          * filler code can execute (basically an 8 or 16-bit component RGB or G
+          * format.)
+          *
+          * NOTE: usr_channels is not used by the read code!  (This has led to
+          * confusion in the past.)  The filler is only used in the read code.
+          */
+         png_ptr->filler = (png_uint_16)filler;
+#     else
+         png_app_error(png_ptr, "png_set_filler not supported on read");
+         PNG_UNUSED(filler); /* not used in the write case */
+         return;
+#     endif
+   }
+
+   else /* write */
+   {
+#     ifdef PNG_WRITE_FILLER_SUPPORTED
+         /* On write the usr_channels parameter must be set correctly at the
+          * start to record the number of channels in the app-supplied data.
+          */
+         switch (png_ptr->color_type)
+         {
+            case PNG_COLOR_TYPE_RGB:
+               png_ptr->usr_channels = 4;
+               break;
+
+            case PNG_COLOR_TYPE_GRAY:
+               if (png_ptr->bit_depth >= 8)
+               {
+                  png_ptr->usr_channels = 2;
+                  break;
+               }
+
+               else
+               {
+                  /* There simply isn't any code in libpng to strip out bits
+                   * from bytes when the components are less than a byte in
+                   * size!
+                   */
+                  png_app_error(png_ptr,
+                     "png_set_filler is invalid for low bit depth gray output");
+                  return;
+               }
+
+            default:
+               png_app_error(png_ptr,
+                  "png_set_filler: inappropriate color type");
+               return;
+         }
+#     else
+         png_app_error(png_ptr, "png_set_filler not supported on write");
+         return;
+#     endif
+   }
+
+   /* Here on success - libpng supports the operation, set the transformation
+    * and the flag to say where the filler channel is.
+    */
    png_ptr->transformations |= PNG_FILLER;
-   png_ptr->filler = (png_uint_16)filler;
 
    if (filler_loc == PNG_FILLER_AFTER)
       png_ptr->flags |= PNG_FLAG_FILLER_AFTER;
 
    else
       png_ptr->flags &= ~PNG_FLAG_FILLER_AFTER;
-
-   /* This should probably go in the "do_read_filler" routine.
-    * I attempted to do that in libpng-1.0.1a but that caused problems
-    * so I restored it in libpng-1.0.2a
-   */
-
-   if (png_ptr->color_type == PNG_COLOR_TYPE_RGB)
-   {
-      png_ptr->usr_channels = 4;
-   }
-
-   /* Also I added this in libpng-1.0.2a (what happens when we expand
-    * a less-than-8-bit grayscale to GA?) */
-
-   if (png_ptr->color_type == PNG_COLOR_TYPE_GRAY && png_ptr->bit_depth >= 8)
-   {
-      png_ptr->usr_channels = 2;
-   }
 }
 
 /* Added to libpng-1.2.7 */
@@ -160,7 +207,9 @@ png_set_add_alpha(png_structrp png_ptr, png_uint_32 filler, int filler_loc)
       return;
 
    png_set_filler(png_ptr, filler, filler_loc);
-   png_ptr->transformations |= PNG_ADD_ALPHA;
+   /* The above may fail to do anything. */
+   if (png_ptr->transformations & PNG_FILLER)
+      png_ptr->transformations |= PNG_ADD_ALPHA;
 }
 
 #endif