Cherry pick unused item

[wantehchang]

No description provided.

[wantehchang]

@vrabaud Vincent: FYI.

@vigneshvg @y-guyon Vignesh, Yannis: The two commits in this pull request will be merged as separate commits. The first commit is a revert of #1974. The second commit is a cherry-pick of #1922. You only need to review the second commit.

[y-guyon]

Thank you

[y-guyon]

This is fine because

libavif/src/read.c

Lines 773 to 777 in e6edfd7

	static avifDecoderItem * avifMetaFindItem(avifMeta * meta, uint32_t itemID, avifDiagnostics * diag)
	{
	if (itemID == 0) {
	return NULL;
	}

[wantehchang]

Yes. If newItemID is 0, avifMetaFindItem(colorItem->meta, newItemID) fails and returns NULL. So we will set result to AVIF_RESULT_OUT_OF_MEMORY in the next line. AVIF_RESULT_OUT_OF_MEMORY is not the right error code for this condition, but at least we will fail rather than keep going.

However, if all the item IDs in the colorItem->meta->items array are nonzero and distinct (I believe this is an invariant that we maintain), then newItemID cannot possibly be zero. So we could add assert(newItemID != 0) after the do-while loop to indicate our belief.

Note: I suggested removing && newItemID != 0 from the condition of the do-while loop, but Vincent was uncomfortable with a potential infinite loop if we fail to maintain the invariant.

[wantehchang]

I just realized that my previous comment is wrong:

However, if all the item IDs in the colorItem->meta->items array are nonzero and distinct (I believe this is an invariant that we maintain), then newItemID cannot possibly be zero. [...]

All we need to prove newItemID cannot be zero is that colorItem->meta->items.count < UINT32_MAX - 1. If there are fewer than UINT32_MAX - 1 elements in the colorItem->meta->items array, then there must be a nonzero uint32_t value that is not an item ID in the colorItem->meta->items array.

Since the colorItem->meta->items.count < UINT32_MAX - 1 condition is guaranteed by the check at line 3700, the only way the do-while loop may loop infinitely is that the loop itself has a bug. The do-while loop is short and can be reviewed thoroughly. So I think the && newItemID != 0 check can be safely omitted.

[y-guyon]

Related: #1993