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

[Rettenbacher, Bernhard]

Hi Pau,
 
I have a little error in the description:  The wrong assertion happens, when the string and the value are 0. The return statement in the for-loop will never be reached, so the program continues to the assertion, which will always fail.
 
The error is quite hard to see, but if you build in debug mode, there's no way to overlook it.
 
bye
 
Bernhard

________________________________

Von: clam-devel-bounces at llistes.projectes.lafarga.org im Auftrag von Pau Arumi
Gesendet: Fr 27.07.2007 18:34
An: clam-devel at llistes.projectes.lafarga.org
Betreff: Re: [Clam-devel] [PATCH] Enum in DEBUG-mode



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);

_______________________________________________
Clam-devel mailing list
Clam-devel at llistes.projectes.lafarga.org
https://llistes.projectes.lafarga.org/cgi-bin/mailman/listinfo/clam-devel


-------------- next part --------------
A non-text attachment was scrubbed...
Name: winmail.dat
Type: application/ms-tnef
Size: 6464 bytes
Desc: not available
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20070728/d140fd7d/attachment-0005.bin>