tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <>
Subject Re: svn commit: r948294 - in /tomcat/tc5.5.x/trunk: ./ container/webapps/docs/ jasper/src/share/org/apache/jasper/ jasper/src/share/org/apache/jasper/compiler/ jasper/src/share/org/apache/jasper/security/ jasper/src/share/org/apache/jasper/servlet/
Date Wed, 26 May 2010 16:52:37 GMT
2010/5/26 sebb <>:
>> The "removed" variable is only checked whether it is zero or some
>>  positive value. So, the actual value of the variable before the
>>  increment operation is irrelevant.  It could be written as "removed =
>>  1;" and assignment is atomic.
> In that case, why not do so - or make it a boolean?

If you see the old code of "isRemoved()" -- before this patch -- it
used to check "if (removed > 1)". So, "0", "1", and ">1" were distinct

I agree with you that this private field can now be made boolean.

>>  Are there any observable consequences? If they are, you may file an issue.
> A brief scan of the code suggests not, unless it is possible for the
> counter to overflow.
> But it is confusing to have a method called incrementRemoved() and a
> corresponding counter which is actually being used purely as a flag.

0 -> 1 is an increment, as well as false -> true. ;)

How the counter is implemented should not bother one who calls this
method. I am fine with its name. (I have thought of a pair of
alternatives, but all they are more confusing than the current name).

> May I suggest adding a comment to document this for future maintainers?


Best regards,
Konstantin Kolinko

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message