Menu
▾
▴
#295 png_get_tRNS code looks odd
In ibpng version 1.6.37 , trying to understand the real behaviour of png_get_tRNS (this behaviour is only partially documented) I see in pngget.c the following definition:
png_uint_32 PNGAPI
png_get_tRNS(png_const_structrp png_ptr, png_inforp info_ptr,
png_bytep *trans_alpha, int *num_trans, png_color_16p *trans_color)
{
png_uint_32 retval = 0;
if (png_ptr != NULL && info_ptr != NULL &&
(info_ptr->valid & PNG_INFO_tRNS) != 0)
{
png_debug1(1, "in %s retrieval function", "tRNS");
if (info_ptr->color_type == PNG_COLOR_TYPE_PALETTE)
{
if (trans_alpha != NULL)
{
*trans_alpha = info_ptr->trans_alpha;
retval |= PNG_INFO_tRNS;
}
if (trans_color != NULL)
*trans_color = &(info_ptr->trans_color);
}
else /* if (info_ptr->color_type != PNG_COLOR_TYPE_PALETTE) */
{
if (trans_color != NULL)
{
*trans_color = &(info_ptr->trans_color);
retval |= PNG_INFO_tRNS;
}
if (trans_alpha != NULL)
*trans_alpha = NULL;
}
if (num_trans != NULL)
{
*num_trans = info_ptr->num_trans;
retval |= PNG_INFO_tRNS;
}
}
return (retval);
}
So if I provide a non-null pointer as an input for the get function, its return value will be non-0, even if the png had no tRNS chunk. Is that the desired behaviour?
Should it not be the following instead ?
png_uint_32 PNGAPI
png_get_tRNS(png_const_structrp png_ptr, png_inforp info_ptr,
png_bytep *trans_alpha, int *num_trans, png_color_16p *trans_color)
{
png_uint_32 retval = 0;
if (png_ptr != NULL && info_ptr != NULL &&
(info_ptr->valid & PNG_INFO_tRNS) != 0)
{
png_debug1(1, "in %s retrieval function", "tRNS");
if (info_ptr->color_type == PNG_COLOR_TYPE_PALETTE)
{
if (trans_alpha != NULL && num_trans != NULL)
{
*trans_alpha = info_ptr->trans_alpha;
*num_trans = info_ptr->num_trans;
retval |= PNG_INFO_tRNS;
}
if (trans_color != NULL)
*trans_color = &(info_ptr->trans_color);
}
else /* if (info_ptr->color_type != PNG_COLOR_TYPE_PALETTE) */
{
if (trans_color != NULL)
{
*trans_color = &(info_ptr->trans_color);
retval |= PNG_INFO_tRNS;
}
if (trans_alpha != NULL)
*trans_alpha = NULL;
}
return (retval);
}
Oops, I had missed the following condition in the outermost "if" :
So my claim was wrong, that it will return non-zero in the case I mentioned.
Nevertheless the code still looks odd to me.
Strange, I cannot correct my ticket: according to https://sourceforge.net/p/forge/documentation/Tickets/#moving-and-deleting-tickets there should be an edit button but I cannot find any.