[PATCH] Improve lib/param handling and remove another unused script

[Andrew Bartlett]

On Wed, 2014-01-29 at 13:34 +1300, Garming Sam wrote:
> Hi there,
> 
> So I split the two commits and changed the indentation.
> >
> > You are adding a check for equality of class
> > (LOCAL/GLOBAL/SEPARATOR/NONE). It took me a bit to grasp
> > it: so the offset can be the same for a LOCAL and a GLOBAL
> > parameter, and if they happen to be neighbors in the param
> > table, we might illegally turn option A read only
> > by setting option B on the command line. Nice catch. did
> > it ever happen?
> >
> I think the original reason why they weren't split was because the bug 
> was actually found with dump_a_parameter coincidentally. It did actually 
> happen, which was bizarre and luckily Andrew knew what happened. In 
> dumping a parameter, one parameter somehow lined up with another from 
> the two structures and so it gave me a strange result. So, we removed 
> the check for synonyms (which looked at the previous offset) and then we 
> found it could happen somewhere else.
> 
> > But one additional though:
> > Here (already previously) the direct neighbors of the current parameter
> > are checked. Is that assumption always safe? In theory this
> > aliases need not be neighbors in the parm_table, right?
> > Shouldn't we play it safe and walk the whole table and move
> > the class and offset checks into the body of the loop?
> >
> It seems to be what is expected of the table. If we ever actually 
> managed to generate the param table, we wouldn't have to worry about it. 
> It could be made more lenient I guess.

The reason it doesn't loop over the whole table is to keep the
complexity at O(n) rather than O(n^2).  With over 300 parameters this is
important, and as the layout requirement has been strictly observed
since synonyms were implemented.

> > But you might explain a bit more in the commit msg:
> > This was supposed to protect from something in the past
> > what changed that makes this protection not necessary any more?
> > Or put differently, do we have parameters starting with "-" now?
> >
> I think the major motivation for this change was really just that -valid 
> was the only thing that was actually hidden. There doesn't seem to be 
> much reason for hiding it basically internally, with there already a 
> check for the prefix in other places. When I made the change, I think 
> Andrew simply told me that the prefix was something that didn't really 
> take off. Should it be flagged in some other way? If anyone has any 
> suggestions, feel free to say.

You are correct, the reason for the change was that it was duplicated
for the 'dump' case, yet stopped the python bindings and the testparm
--parameter-name code from reading the value of -valid.  The new docs.py
test found the special case, because it checked for every documented
parameter, including -valid. 

Thanks for updating the patches!

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba