[Freeipa-devel] [PATCH] 212 Fix integer validation when boundary value is empty string

[Petr Vobornik]

On 09/18/2012 04:45 AM, Endi Sukma Dewata wrote:
> On 9/11/2012 10:09 AM, Petr Vobornik wrote:
>> There was an error in number validation check. If boundary value was an
>> empty string, validation of a number always failed. This patch fixes the
>> problem by not performing the check in these cases.
>>
>> Basic unit tests for IPA.metadata_validator created.
>
> ACK. Some comments:

Updated patch attached.

>
> 1. Instead of IPA.not_defined() it might be better to call it
> IPA.defined() to avoid double negations like this:
>
>    if (!IPA.not_defined(metadata.minvalue, true) ...
>

Function renamed, logic negated. Unit tests for IPA.defined added.

> 2. The check_empty_str probably could made optional and the value would
> be true by default. It will make the code cleaner.
>

I wanted the function to be more general - used somewhere else in a 
future. In general, I consider empty string as a defined value so for me 
'false' is a good default value of the argument. I agree that in this 
case it would look cleaner.
-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0212-1-Fix-integer-validation-when-boundary-value-is-empty-.patch
Type: text/x-patch
Size: 9966 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120918/d78ea4f8/attachment.bin>