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

Garming Sam garming at catalyst.net.nz
Tue Jan 28 17:34:42 MST 2014


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.

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



Cheers,

Garming Sam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-param-fix-an-offset-bug-in-lpcfg_set_cmdline.patch
Type: text/x-patch
Size: 1547 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-param-remove-unnecessary-checks-in-dump-a-parameter.patch
Type: text/x-patch
Size: 1224 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-docs-update-XInclude-year-to-conform-with-current-st.patch
Type: text/x-patch
Size: 1083 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-docs-remove-the-file-prefix-from-included-path-names.patch
Type: text/x-patch
Size: 1078 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-lib-param-fix-copy-service-to-include-the-case-for-P.patch
Type: text/x-patch
Size: 868 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-param-don-t-ignore-some-parameters-when-performing-m.patch
Type: text/x-patch
Size: 1599 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-docs-Remove-find_missing_manpages-replaced-by-docs.p.patch
Type: text/x-patch
Size: 3798 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-docs-Remove-unused-parameters.global.xml-and-paramet.patch
Type: text/x-patch
Size: 2030 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-lib-param-Fix-copy_service-to-handle-BOOLREV.patch
Type: text/x-patch
Size: 710 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140129/35f9b054/attachment-0008.bin>


More information about the samba-technical mailing list