Minor write bug-fixes, remove unimplemented code

A debug() assert fired if windowBits was set to 8 for the Huffman only and
no-compression cases.  This commit changes it to do some extra checking.  Remove
unreachable code in pz_default_settings, eliminate a spurious warning in pngcp
for small files.

Signed-off-by: John Bowler <jbowler@acm.org>
This commit is contained in:
John Bowler 2016-05-06 14:35:35 -07:00
parent e966fac5ac
commit 4cc89fb733
4 changed files with 70 additions and 168 deletions

View File

@ -328,6 +328,7 @@ struct display
int bpp;
png_byte ct;
int no_warnings; /* Do not output libpng warnings */
int min_windowBits; /* The windowBits range is 8..8 */
/* Options handling */
png_uint_32 results; /* A mask of errors seen */
@ -389,6 +390,7 @@ display_init(struct display *dp)
dp->read_pp = NULL;
dp->ip = NULL;
dp->write_pp = NULL;
dp->min_windowBits = -1; /* this is an OPTIND, so -1 won't match anything */
# if PNG_LIBPNG_VER < 10700
dp->text_ptr = NULL;
dp->num_text = 0;
@ -775,8 +777,12 @@ push_opt(struct display *dp, unsigned int sp, png_byte opt, int search)
if (opt_list_end(dp, opt, entry))
{
dp->stack[sp].end = 1;
display_log(dp, APP_WARNING, "%s: only testing one value",
options[opt].name);
/* Skip the warning if pngcp did this itself. See the code in
* set_windowBits_hi.
*/
if (opt != dp->min_windowBits)
display_log(dp, APP_WARNING, "%s: only testing one value",
options[opt].name);
}
else
@ -1194,7 +1200,7 @@ getsearchopts(struct display *dp, const char *opt_str, int *value)
{
/* Changing windowBits for strategies that do not search the window is
* pointless. Huffman-only does not search, RLE only searchs backwards
* one byte, so give that the maximum string length is 258, a windowBits
* one byte, so given that the maximum string length is 258, a windowBits
* of 9 is always sufficient.
*/
if (dp->value[istrat] == Z_HUFFMAN_ONLY)
@ -1841,6 +1847,13 @@ set_windowBits_hi(struct display *dp)
assert(VLNAME(windowBits_IDAT)[--i].name == range_lo);
VLNAME(windowBits_IDAT)[i].value = wb > 8 ? 9 : 8;
/* If wb == 8 then any search has been restricted to just one windowBits
* entry. Record that here to avoid producing a spurious app-level warning
* above.
*/
if (wb == 8)
dp->min_windowBits = OPTIND(dp, windowBits);
}
static int

7
png.h
View File

@ -1226,11 +1226,12 @@ PNG_EXPORT(241, int, png_convert_to_rfc1123_buffer, (char out[29],
#ifdef PNG_CONVERT_tIME_SUPPORTED
/* Convert from a struct tm to png_time */
PNG_EXPORT(24, void, png_convert_from_struct_tm, (png_timep ptime,
const struct tm * ttime));
PNG_EXPORT(24, PNG_DEPRECATED void, png_convert_from_struct_tm,
(png_timep ptime, const struct tm * ttime));
/* Convert from time_t to png_time. Uses gmtime() */
PNG_EXPORT(25, void, png_convert_from_time_t, (png_timep ptime, time_t ttime));
PNG_EXPORT(25, PNG_DEPRECATED void, png_convert_from_time_t, (png_timep ptime,
time_t ttime));
#endif /* CONVERT_tIME */
#ifdef PNG_READ_EXPAND_SUPPORTED

View File

@ -1211,10 +1211,11 @@ PNG_INTERNAL_FUNCTION(void,png_write_start_IDAT,(png_structrp png_ptr),
*/
enum
{
png_row_end =0x1U, /* This is the last block in the row */
png_pass_first_row =0x2U, /* This is the first row in a pass */
png_pass_last_row =0x4U, /* This is the last row in a pass */
png_pass_last =0x8U /* This is the last pass in the image */
png_pass_last =0x1U, /* This is the last pass in the image */
png_pass_last_row =0x2U, /* This is the last row in a pass */
png_pass_first_row =0x4U, /* This is the first row in a pass */
png_row_end =0x8U, /* This is the last block in the row */
png_no_row_info =0x0U /* Placeholder */
/* A useful macro; return true if this is the last block of the last row in
* the image.

View File

@ -850,17 +850,6 @@ typedef struct png_zlib_state
*/
png_bytep previous_write_row;
png_alloc_size_t write_row_size; /* Actual size of the buffers */
png_alloc_size_t write_buffer_limit; /* Limit on total allocation */
# ifdef PNG_SELECT_FILTER_SUPPORTED
png_bytep *saved_rows; /* Rows saved by libpng */
png_bytep saved_filters; /* Filters for those rows */
png_alloc_size_t saved_offset; /* First saved bytes in: */
png_uint_32 saved_first; /* First buffer in use */
png_uint_32 maximum_saved_rows; /* Size of the above buffers */
png_uint_32 current_saved_rows;
# endif /* SELECT_FILTER */
png_uint_32 save_row_count; /* Total number to be buffered */
# define SAVE_ROW_COUNT_UNSET 0xFFFFFFFFU
@ -905,7 +894,6 @@ png_create_zlib_state(png_structrp png_ptr)
png_zlib_compress_init(png_ptr, &ps->s);
# ifdef PNG_WRITE_FILTER_SUPPORTED
ps->write_buffer_limit = PNG_COMPRESSION_BUFFER_LIMIT;
ps->save_row_count = SAVE_ROW_COUNT_UNSET;
# endif /* WRITE_FILTER */
# ifdef PNG_WRITE_FLUSH_SUPPORTED
@ -925,40 +913,13 @@ png_deflate_release(png_structrp png_ptr, png_zlib_statep ps, int check)
{
# ifdef PNG_WRITE_FILTER_SUPPORTED
/* Free any mode-specific data that is owned here: */
# ifdef PNG_SELECT_FILTER_SUPPORTED
if (ps->saved_rows != NULL)
{
/* If the saved row array was allocated the previous row pointer was
* *not* allocated. Free any saved row memory:
*/
png_bytep *save = ps->saved_rows;
png_uint_32 i = ps->maximum_saved_rows;
ps->saved_rows = NULL;
while (i > 0)
png_free(png_ptr, save[--i]);
png_free(png_ptr, save);
if (ps->saved_filters != NULL)
{
png_bytep p = ps->saved_filters;
ps->saved_filters = NULL;
png_free(png_ptr, p);
}
}
else
# else /* !SELECT_FILTER */
if (ps->previous_write_row != NULL)
{
/* No saved rows, so the previous row buffer was allocated: */
png_bytep p = ps->previous_write_row;
ps->previous_write_row = NULL;
png_free(png_ptr, p);
}
# endif /* !SELECT_FILTER */
if (ps->previous_write_row != NULL)
{
/* No saved rows, so the previous row buffer was allocated: */
png_bytep p = ps->previous_write_row;
ps->previous_write_row = NULL;
png_free(png_ptr, p);
}
# endif /* WRITE_FILTER */
/* The main z_stream opaque pointer needs to remain set to png_ptr; it is
@ -1048,27 +1009,41 @@ png_deflate_destroy(png_structrp png_ptr)
static void
fix_cinfo(png_zlib_statep ps, png_bytep data, png_alloc_size_t data_size)
{
/* Do this if the data is 256 bytes or less and the CINFO field is '1',
* meaning windowBits of 8. The first byte of the stream is the CMF value,
* CINFO is in the upper four bits.
/* Do this if the CINFO field is '1', meaning windowBits of 9. The first
* byte of the stream is the CMF value, CINFO is in the upper four bits.
*
* If zlib didn't futz with the value then it should match the value in
* pz_current; check this is debug. (See below for why this works in the
* pz_default_settings call.)
*/
# define png_ptr png_voidcast(png_const_structrp, ps->s.zs.opaque)
if (data_size <= 256U && data[0] == 0x18U &&
if (data[0] == 0x18U &&
pz_get(ps, current, windowBits, 0) == 8 /* i.e. it was requested */)
{
unsigned int d1;
data[0] = 0x08U;
/* The header checksum must be fixed too. The FCHECK (low 5 bits) make
* CMF.FLG a multiple of 31:
/* Double check this here; the fixup only works if the data was 256 bytes
* or shorter *or* the window is never used. For safety repeat the checks
* done in pz_default_settings; technically we should be able to just skip
* this test.
*
* TODO: set a 'fixup' flag in zlib_state to make this quicker?
*/
d1 = data[1] & 0xE0U; /* top three bits */
d1 += 31U - (0x0800U + d1) % 31U;
data[1] = PNG_BYTE(d1);
if (data_size <= 256U ||
pz_get(ps, current, strategy, Z_RLE) == Z_HUFFMAN_ONLY ||
pz_get(ps, current, level, 1) == Z_NO_COMPRESSION)
{
unsigned int d1;
data[0] = 0x08U;
/* The header checksum must be fixed too. The FCHECK (low 5 bits) make
* CMF.FLG a multiple of 31:
*/
d1 = data[1] & 0xE0U; /* top three bits */
d1 += 31U - (0x0800U + d1) % 31U;
data[1] = PNG_BYTE(d1);
}
else /* pz_default_settings is expected to guarantee the above */
NOT_REACHED;
}
else
@ -1153,7 +1128,11 @@ pz_default_settings(png_uint_32 settings, png_uint_32 owner,
* Z_DEFAULT_COMPRESSION is, in fact, level 6 so Mark seems to
* concur.
*/
zlib_level = png_level;
if (png_level < 9)
zlib_level = png_level;
else /* PNG compression level 10; the ridiculous level */
zlib_level = 9;
break;
}
@ -1231,15 +1210,14 @@ pz_default_settings(png_uint_32 settings, png_uint_32 owner,
*/
switch (zlib_level)
{
case 0: /* no compression */
test_size = 0U;
break;
case 1: case 2: case 3:
test_size = data_size / 8U;
break;
default:
/* This includes, implicitly, ZLIB_NO_COMPRESSION, but that
* was eliminated in the 'if' above.
*/
test_size = data_size / 4U;
break;
@ -3987,82 +3965,6 @@ png_write_start_IDAT(png_structrp png_ptr)
if (write_row_size == 0U) /* row too large to buffer */
ps->save_row_count = 0U;/* no buffering; filters will be NONE or SUB */
# ifdef PNG_SELECT_FILTER_SUPPORTED
else if ((src == SAVE_ROW_COUNT_UNSET && (mask & (mask-1)) != 0U)
/* default to selection */ ||
(src < SAVE_ROW_COUNT_UNSET && src >= 2U)
/* app turned selection on */)
{
/* Filter selection support required. save_row_count is a measure
* of the amount of PNG encoded but uncompressed data to use in
* making a filter selection plus one row for previous row filter
* handling.
*
* The amount of data buffered is limited by all of:
*
* 1) ps->write_buffer_limit
* 2) ps->save_row_count
* 3) png_ptr->height
*/
if (src > png_ptr->height)
src = png_ptr->height;
if (src > 2U)
{
/* Check ps->write_buffer_limit too, this requires more
* calculation so is done last. Notice that write_buffer_limit
* is the maximum number of bytes the deflate implementation
* requires, so the calculation here is to find out the minimum
* number of buffers to accomodate that limit.
*
* The calculation required is:
*
* write_buffer_limit / PNG-row-bytes
*
* Where PNG-row-bytes is the actual PNG row bytes including the
* filter byte. Unfortunately the latter calculation can
* overflow a 32-bit unsigned value.
*/
const unsigned int pixel_bits = PNG_PIXEL_DEPTH(*png_ptr);
# if PNG_COMPRESSION_BUFFER_LIMIT > 0xFFFFFFFFU / 8U
# error LibPNG COMPRESSION BUFFER LIMIT setting too large
# endif /* COMPRESSION_BUFFER_LIMIT test */
/* Therefore this will not overflow a png_uint_32: */
const png_alloc_size_t pixel_limit =
(8U*ps->write_buffer_limit + pixel_bits - 1U)/pixel_bits;
if (pixel_limit < png_ptr->width)
src = 2U; /* clamp to the minimum required */
else
{
/* pixel_limit >= png_ptr->width, so this cannot overflow: */
const png_alloc_size_t rb =
PNG_ROWBYTES(pixel_bits, png_ptr->width);
const png_alloc_size_t row_limit =
(ps->write_buffer_limit + rb) / (rb+1U);
if (row_limit < src)
src = (png_uint_32)/*SAFE*/row_limit;
if (src < 2U)
src = 2U;
}
}
/* save_row_count will be just 1 for a single pixel high image, so
* an extra flag is required to record that filter selection is
* being done.
*/
ps->save_row_count = src;
ps->do_select = 1U;
}
else
# endif /* SELECT_FILTER */
/* If unset no filter selection is required (or, maybe, available),
* respect the app setting if the buffering has been set to 'off' or
* 'previous row':
@ -4188,13 +4090,6 @@ png_write_png_data(png_structrp png_ptr, png_bytep prev_pixels,
affirm(ps != NULL);
# ifdef PNG_SELECT_FILTER_SUPPORTED
/* The data will be buffered for later use. */
if (ps->do_select)
png_write_buffer_row(ps, unfiltered_row, x, width, row_info_flags);
else /* no filter selection */
# endif /* SELECT_FILTER */
{
const unsigned int bpp = png_ptr->row_output_pixel_depth;
const unsigned int row_bits = width * bpp;
@ -4239,19 +4134,11 @@ png_write_png_rows(png_structrp png_ptr, png_const_bytep *rows,
affirm(ps != NULL);
# ifdef PNG_SELECT_FILTER_SUPPORTED
if (ps->do_select)
select_png_rows(ps, rows, num_rows);
/* Set the filter to use: */
png_write_start_row(ps);
else
# endif /* !SELECT_FILTER */
{
/* Set the filter to use: */
png_write_start_row(ps);
/* Now write all the rows with the same filter: */
write_png_rows(png_ptr, rows, num_rows);
}
/* Now write all the rows with the same filter: */
write_png_rows(png_ptr, rows, num_rows);
}
#else /* !WRITE_FILTER */
void /* PRIVATE */