[Freeipa-devel] [PATCH 48/48] Ticket #1879 - IPAdmin undefined anonymous parameter lists

[Jan Cholasta]

On 26.9.2011 21:52, John Dennis wrote:
> The IPAdmin class in ipaserver/ipaldap.py has methods with anonymous
> undefined parameter lists.
>
> For example:
>
>      def getList(self,*args):
>
> In Python syntax this means you can call getList with any positional
> parameter list you want.
>
> This is bad because:
>
> 1) It's not true, *args gets passed to an ldap function with a well
> defined parameter list, so you really do have to call it with a
> defined parameter list. *args will let you pass anything, but once it
> gets passed to the ldap function it will blow up if the parameters do
> not match (what parameters are those you're wondering? see item 2).
>
> 2) The programmer does not know what the valid parameters are unless
> they are defined in the formal parameter list.
>
> 3) Without a formal parameter list automatic documentation generators
> cannot produce API documentation (see item 2)
>
> 4) The Python interpreter cannot validate the parameters being passed
> because there is no formal parameter list. Note, Python does not
> validate the type of parameters, but it does validate the correct
> number of postitional parameters are passed and only defined keyword
> parameters are passed. Bypassing the language support facilities leads
> to programming errors.
>
> 5) Without a formal parameter list program checkers such as pylint
> cannot validate the program which leads to progamming errors.
>
> 6) Without a formal parameter list which includes default keyword
> parameters it's not possible to use keyword arguments nor to know what
> their default values are (see item 2). One is forced to pass a keyword
> argument as a positional argument, plus you must then pass every
> keyword argument between the end of the positional argument list and
> keyword arg of interest even of the other keyword arguments are not of
> interest. This also demands you know what the default value of the
> intermediate keyword arguments are (see item 2) and hope they don't
> change.
>
> Also the *args anonymous tuple get passed into the error handling code
> so it can report what the called values were. But because the tuple is
> anonymous the error handler cannot not describe what it was passed. In
> addition the error handling code makes assumptions about the possible
> contents of the anonymous tuple based on current practice instead of
> actual defined values. Things like "if the number of items in the
> tuple is 2 or less then the first tuple item must be a dn
> (Distinguished Name)" or "if the number of items in the tuple is
> greater than 2 then the 3rd item must be an ldap search filter". These
> are constructs which are not robust and will fail at some point in the
> future.
>
> This patch also fixes the use of IPAdmin.addEntry(). It was sometimes
> being called with (dn, modlist), sometimes a Entry object, or
> sometimes a Entity object. Now it's always called with either a Entry
> or Entity object and IPAdmin.addEntry() validates the type of the
> parameter passed.
>
> --
> John Dennis<jdennis at redhat.com>
>
> Looking to carve out IT costs?
> www.redhat.com/carveoutcosts/
>

ACK.

Honza

-- 
Jan Cholasta