[Clam-devel] [PATCH] Enum in DEBUG-mode

[Pau Arumi]

En/na Rettenbacher, Bernhard ha escrit:
> Dear CLAM-Developers,
> 
> About 1 or 2 months ago, I sent you some patches, one of them was for EAudioFileFormat::FormatFromFilename. You rejected it, because I did break your rules how to handle arrays of structures. Now - as I made a build of the current sources - we had an eye again on this problem:
> 
> When calling FormatFromFilename, an array of Enums, containing the possible file extensions, is created. This array is terminated by a {0,0}-pair. In DEBUG-mode, this causes an assertion in Enum::SetValue, because a null string is the stop criterion in the for-loop. So, if the value, you want to set is 0, the code after the for-loop is executed, which is CLAM_ASSERT(false,...). In RELEASE mode, this loop and the assertion are never executed.
> 
> Our solution replaces the for loop in Enum::SetValue by a do-while loop, which takes the {0,0}-pair in account.

I think the code have the intended behaviour. But maybe I don't
understand your claim.

Enum::SetValue(v) stores v into a member of the enum.

In debug we want to check (assert) that v is one of the valid
options of the enum. In release we don't: just assign v to the
member.

You say: if the value you want to set is zero (SetValue(0)) it will
assert.
If the enum is defined with 0 as a possible value then it should
not assert. For example
{
	{"wav",  0},
	{0,0}
}
Since the iteration is finished when mEnumValues[i].name==0 not
.value==0}


On the other hand the patch version have a problem. Take this enum:
{
	{"wav",  1},
	{0,0}
}
SetValue(0) will not assert when it should since 0 is not an
accepted value.

Cheers
Pau

-----

Current code:

for (int i = 0; mEnumValues[i].name; i++)
	if (v==mEnumValues[i].value) {
		mValue = v;
		return;
	}


Patch:

int i = 0;
do {
	if (v==mEnumValues[i].value) {
		mValue = v;
		return;
	}
} while (mEnumValues[i++].name);