Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pngrutil.c: Delete dead code in png_handle_PLTE() #484

Open
wants to merge 1 commit into
base: libpng16
Choose a base branch
from
Open

pngrutil.c: Delete dead code in png_handle_PLTE() #484

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 1 addition & 33 deletions pngrutil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,39 +1014,7 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr, png_uint_32 length)
* have an RGB image, the PLTE can be considered ancillary, so
* we will act as though it is.
*/
#ifndef PNG_READ_OPT_PLTE_SUPPORTED
if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE)
#endif
{
png_crc_finish(png_ptr, (png_uint_32) (length - (unsigned int)num * 3));
}

#ifndef PNG_READ_OPT_PLTE_SUPPORTED
else if (png_crc_error(png_ptr) != 0) /* Only if we have a CRC error */
{
/* If we don't want to use the data from an ancillary chunk,
* we have two options: an error abort, or a warning and we
* ignore the data in this chunk (which should be OK, since
* it's considered ancillary for a RGB or RGBA image).
*
* IMPLEMENTATION NOTE: this is only here because png_crc_finish uses the
* chunk type to determine whether to check the ancillary or the critical
* flags.
*/
if ((png_ptr->flags & PNG_FLAG_CRC_ANCILLARY_USE) == 0)
{
if ((png_ptr->flags & PNG_FLAG_CRC_ANCILLARY_NOWARN) != 0)
return;

else
png_chunk_error(png_ptr, "CRC error");
}

/* Otherwise, we (optionally) emit a warning and use the chunk. */
else if ((png_ptr->flags & PNG_FLAG_CRC_ANCILLARY_NOWARN) == 0)
png_chunk_warning(png_ptr, "CRC error");
}
#endif
png_crc_finish(png_ptr, (png_uint_32) (length - (unsigned int)num * 3));
Copy link
Contributor Author

@wantehchang wantehchang Jun 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1) Please check if the comment block at lines 1012-1016 should be updated.

(2) I assume there is no bug and therefore the unreachable code can be safely deleted. But perhaps there is a bug and the code should not be unreachable. So please check the code I deleted carefully.

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are both "#ifndef".

Your logic seems to assume the second is "#ifdef"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm they are both #ifndef. I did not assume the second is #ifdef.

Earlier in the function, at lines 952-958, we have:

#ifndef PNG_READ_OPT_PLTE_SUPPORTED
   if (png_ptr->color_type != PNG_COLOR_TYPE_PALETTE)
   {
      png_crc_finish(png_ptr, length);
      return;
   }
#endif

Suppose #ifndef PNG_READ_OPT_PLTE_SUPPORTED is true. If we reach here, the png_ptr->color_type == PNG_COLOR_TYPE_PALETTE condition at line 1018 of the original code must be true, so the else if block at lines 1025-1048 of the original code will never be taken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that the comment block at lines 1012-1016 describes the original code. So the comment block should be deleted (or updated) in this pull request.


/* TODO: png_set_PLTE has the side effect of setting png_ptr->palette to its
* own copy of the palette. This has the side effect that when png_start_row
Expand Down