[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