[libpng17] Fixed GCC unsigned int->float warnings. Various versions of GCC

seem to generate warnings when an unsigned value is implicitly
    converted to double. This is probably a GCC bug but this change
    avoids the issue by explicitly converting to (int) where safe.
  Free all allocated memory in pngimage. The file buffer cache was left
    allocated at the end of the program, harmless but it causes memory
    leak reports from clang.
  Fixed array size calculations to avoid warnings. At various points
    in the code the number of elements in an array is calculated using
    sizeof.  This generates a compile time constant of type (size_t) which
    is then typically assigned to an (unsigned int) or (int). Some versions
    of GCC on 64-bit systems warn about the apparent narrowing, even though
    the same compiler does apparently generate the correct, in-range,
    numeric constant.  This adds appropriate, safe, casts to make the
    warnings go away.

ANNOUNCE

@@ -1,5 +1,5 @@
 
-Libpng 1.7.0beta40 - November 5, 2014
+Libpng 1.7.0beta40 - November 6, 2014
 
 This is not intended to be a public release.  It will be replaced
 within a few weeks by a public version or by another test version.
@@ -620,13 +620,28 @@ Version 1.7.0beta38 [October 17, 2014]
 Version 1.7.0beta39 [November 1, 2014]
   Ported cosmetic changes from libpng-1.6.15beta02.
 
-Version 1.7.0beta40 [November 5, 2014]
+Version 1.7.0beta40 [November 6, 2014]
   Made a one-line revision to configure.ac to support ARM on aarch64
     (bug report by Marcin Juszkiewicz, fix by John Bowler).
   Use png_get_libpng_ver(NULL) instead of PNG_LIBPNG_VER_STRING in
     example.c, pngtest.c, and applications in the contrib directory.
   Avoid out-of-bounds memory access in png_user_version_check().
   Simplified and future-proofed png_user_version_check().
+  Fixed GCC unsigned int->float warnings. Various versions of GCC
+    seem to generate warnings when an unsigned value is implicitly
+    converted to double. This is probably a GCC bug but this change
+    avoids the issue by explicitly converting to (int) where safe.
+  Free all allocated memory in pngimage. The file buffer cache was left
+    allocated at the end of the program, harmless but it causes memory
+    leak reports from clang.
+  Fixed array size calculations to avoid warnings. At various points
+    in the code the number of elements in an array is calculated using
+    sizeof.  This generates a compile time constant of type (size_t) which
+    is then typically assigned to an (unsigned int) or (int). Some versions
+    of GCC on 64-bit systems warn about the apparent narrowing, even though
+    the same compiler does apparently generate the correct, in-range,
+    numeric constant.  This adds appropriate, safe, casts to make the
+    warnings go away.
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

CHANGES

@@ -4909,13 +4909,28 @@ Version 1.7.0beta38 [October 17, 2014]
 Version 1.7.0beta39 [November 1, 2014]
   Ported cosmetic changes from libpng-1.6.15beta02.
 
-Version 1.7.0beta40 [November 5, 2014]
+Version 1.7.0beta40 [November 6, 2014]
   Made a one-line revision to configure.ac to support ARM on aarch64
     (bug report by Marcin Juszkiewicz, fix by John Bowler).
   Use png_get_libpng_ver(NULL) instead of PNG_LIBPNG_VER_STRING in
     example.c, pngtest.c, and applications in the contrib directory.
   Avoid out-of-bounds memory access in png_user_version_check().
   Simplified and future-proofed png_user_version_check().
+  Fixed GCC unsigned int->float warnings. Various versions of GCC
+    seem to generate warnings when an unsigned value is implicitly
+    converted to double. This is probably a GCC bug but this change
+    avoids the issue by explicitly converting to (int) where safe.
+  Free all allocated memory in pngimage. The file buffer cache was left
+    allocated at the end of the program, harmless but it causes memory
+    leak reports from clang.
+  Fixed array size calculations to avoid warnings. At various points
+    in the code the number of elements in an array is calculated using
+    sizeof.  This generates a compile time constant of type (size_t) which
+    is then typically assigned to an (unsigned int) or (int). Some versions
+    of GCC on 64-bit systems warn about the apparent narrowing, even though
+    the same compiler does apparently generate the correct, in-range,
+    numeric constant.  This adds appropriate, safe, casts to make the
+    warnings go away.
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

contrib/libtests/pngimage.c

@@ -337,6 +337,9 @@ validate_T(void)
  *    In both cases the file data is held in a linked list of buffers - not all
  *    of these are in use at any time.
  */
+#define NEW(type) ((type *)malloc(sizeof (type)))
+#define DELETE(ptr) (free(ptr))
+
 struct buffer_list
 {
    struct buffer_list *next;         /* next buffer in list */
@@ -361,6 +364,25 @@ buffer_init(struct buffer *buffer)
    buffer->current = NULL;
 }
 
+static void
+buffer_destroy_list(struct buffer_list *list)
+{
+   if (list != NULL)
+   {
+      struct buffer_list *next = list->next;
+      DELETE(list);
+      buffer_destroy_list(next);
+   }
+}
+
+static void
+buffer_destroy(struct buffer *buffer)
+{
+   struct buffer_list *list = buffer->first.next;
+   buffer_init(buffer);
+   buffer_destroy_list(list);
+}
+
 #ifdef PNG_WRITE_SUPPORTED
 static void
 buffer_start_write(struct buffer *buffer)
@@ -390,8 +412,6 @@ get_buffer(png_structp pp)
    return (struct buffer*)png_get_io_ptr(pp);
 }
 
-#define NEW(type) ((type *)malloc(sizeof (type)))
-
 static struct buffer_list *
 buffer_extend(struct buffer_list *current)
 {
@@ -598,6 +618,17 @@ display_clean(struct display *dp)
    dp->results = 0; /* reset for next time */
 }
 
+static void
+display_destroy(struct display *dp)
+{
+    /* Release any memory held in the display. */
+#  ifdef PNG_WRITE_SUPPORTED
+      buffer_destroy(&dp->written_file);
+#  endif
+
+   buffer_destroy(&dp->original_file);
+}
+
 static struct display *
 get_dp(png_structp pp)
    /* The display pointer is always stored in the png_struct error pointer */
@@ -1605,6 +1636,9 @@ main(const int argc, const char * const * const argv)
          display_clean(&d);
       }
 
+      /* Release allocated memory */
+      display_destroy(&d);
+
       return errors != 0;
    }
 }

contrib/libtests/pngvalid.c

@@ -157,6 +157,13 @@ define_exception_type(struct png_store*);
    &(ps)->exception_context
 #define context(ps,fault) anon_context(ps); png_store *fault
 
+/* This macro returns the number of elements in an array as an (unsigned int),
+ * it is necessary to avoid the inability of certain versions of GCC to use
+ * the value of a compile-time constant when performing range checks.  It must
+ * be passed an array name.
+ */
+#define ARRAY_SIZE(a) ((unsigned int)((sizeof (a))/(sizeof (a)[0])))
+
 /******************************* UTILITIES ************************************/
 /* Error handling is particularly problematic in production code - error
  * handlers often themselves have bugs which lead to programs that detect
@@ -4106,7 +4113,7 @@ make_errors(png_modifier* PNG_CONST pm, png_byte PNG_CONST colour_type,
          standard_name(name, sizeof name, 0, colour_type, 1<<bdlo, 0,
             interlace_type, 0, 0, 0);
 
-         for (test=0; test<(sizeof error_test)/(sizeof error_test[0]); ++test)
+         for (test=0; test<ARRAY_SIZE(error_test); ++test)
          {
             make_error(&pm->this, colour_type, DEPTH(bdlo), interlace_type,
                test, name);
@@ -10084,12 +10091,12 @@ int main(int argc, char **argv)
 
    /* Store the test gammas */
    pm.gammas = gammas;
-   pm.ngammas = (sizeof gammas) / (sizeof gammas[0]);
+   pm.ngammas = ARRAY_SIZE(gammas);
    pm.ngamma_tests = 0; /* default to off */
 
    /* And the test encodings */
    pm.encodings = test_encodings;
-   pm.nencodings = (sizeof test_encodings) / (sizeof test_encodings[0]);
+   pm.nencodings = ARRAY_SIZE(test_encodings);
 
    pm.sbitlow = 8U; /* because libpng doesn't do sBIT below 8! */
 

png.c

@@ -691,13 +691,13 @@ png_get_copyright(png_const_structrp png_ptr)
 #else
 #  ifdef __STDC__
    return PNG_STRING_NEWLINE \
-     "libpng version 1.7.0beta40 - November 5, 2014" PNG_STRING_NEWLINE \
+     "libpng version 1.7.0beta40 - November 6, 2014" PNG_STRING_NEWLINE \
      "Copyright (c) 1998-2014 Glenn Randers-Pehrson" PNG_STRING_NEWLINE \
      "Copyright (c) 1996-1997 Andreas Dilger" PNG_STRING_NEWLINE \
      "Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc." \
      PNG_STRING_NEWLINE;
 #  else
-      return "libpng version 1.7.0beta40 - November 5, 2014\
+      return "libpng version 1.7.0beta40 - November 6, 2014\
       Copyright (c) 1998-2014 Glenn Randers-Pehrson\
       Copyright (c) 1996-1997 Andreas Dilger\
       Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.";
@@ -3621,7 +3621,31 @@ png_gamma_8bit_correct(unsigned int value, png_fixed_point gamma_val)
    if (value > 0 && value < 255)
    {
 #     ifdef PNG_FLOATING_ARITHMETIC_SUPPORTED
-         double r = floor(255*pow(value/255.,gamma_val*.00001)+.5);
+         /* 'value' is unsigned, ANSI-C90 requires the compiler to correctly
+          * convert this to a floating point value.  This includes values that
+          * would overflow if 'value' were to be converted to 'int'.
+          *
+          * Apparently GCC, however, does an intermediate convertion to (int)
+          * on some (ARM) but not all (x86) platforms, possibly because of
+          * hardware FP limitations.  (E.g. if the hardware convertion always
+          * assumes the integer register contains a signed value.)  This results
+          * in ANSI-C undefined behavior for large values.
+          *
+          * Other implementations on the same machine might actually be ANSI-C90
+          * conformant and therefore compile spurious extra code for the large
+          * values.
+          *
+          * We can be reasonably sure that an unsigned to float convertion
+          * won't be faster than an int to float one.  Therefore this code
+          * assumes responsibility for the undefined behavior, which it knows
+          * can't happen because of the check above.
+          *
+          * Note the argument to this routine is an (unsigned int) because, on
+          * 16-bit platforms, it is assigned a value which might be out of
+          * range for an (int); that would result in undefined behavior in the
+          * caller if the *argument* ('value') were to be declared (int).
+          */
+         double r = floor(255*pow((int)/*SAFE*/value/255.,gamma_val*.00001)+.5);
          return (png_byte)r;
 #     else
          png_int_32 lg2 = png_log8bit(value);
@@ -3644,7 +3668,13 @@ png_gamma_16bit_correct(unsigned int value, png_fixed_point gamma_val)
    if (value > 0 && value < 65535)
    {
 #     ifdef PNG_FLOATING_ARITHMETIC_SUPPORTED
-         double r = floor(65535*pow(value/65535.,gamma_val*.00001)+.5);
+         /* The same (unsigned int)->(double) constraints apply here as above,
+          * however in this case the (unsigned int) to (int) convertion can
+          * overflow on an ANSI-C90 compliant system so the cast needs to ensure
+          * that this is not possible.
+          */
+         double r = floor(65535.*pow((png_int_32)value/65535.,
+                     gamma_val*.00001)+.5);
          return (png_uint_16)r;
 #     else
          png_int_32 lg2 = png_log16bit(value);

pngread.c

@@ -1618,7 +1618,7 @@ png_image_skip_unused_chunks(png_structrp png_ptr)
 
        /* But do not ignore image data handling chunks */
        png_set_keep_unknown_chunks(png_ptr, PNG_HANDLE_CHUNK_AS_DEFAULT,
-         chunks_to_process, (sizeof chunks_to_process)/5);
+         chunks_to_process, (int)/*SAFE*/(sizeof chunks_to_process)/5);
     }
 }
 

pngset.c

@@ -1373,7 +1373,7 @@ png_set_keep_unknown_chunks(png_structrp png_ptr, int keep,
       };
 
       chunk_list = chunks_to_ignore;
-      num_chunks = (sizeof chunks_to_ignore)/5;
+      num_chunks = (unsigned int)/*SAFE*/(sizeof chunks_to_ignore)/5U;
    }
 
    else /* num_chunks_in > 0 */