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

Endi Sukma Dewata edewata at redhat.com
Tue Sep 18 16:47:31 UTC 2012


On 9/18/2012 6:36 AM, Petr Vobornik wrote:
> Updated patch attached.

ACK.

>> 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.

OK. It's also possible to use different methods, e.g. IPA.is_null() and 
IPA.is_empty().

-- 
Endi S. Dewata




More information about the Freeipa-devel mailing list