RFC: reduce number of alias parameters

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

RFC: reduce number of alias parameters

Samba - samba-technical mailing list
Hi,

I think it's not useful to have a huge number of aliases just to make the
parameters expressable is different humand wordings or spellings. They increase
the code and binary size and more importantly they make config parsing and more
difficult and error prone. For some parameter aliases which are not very
commonly used imho, I'd like to propose to remove them for the next release.
Given that Metze already removed a number of other parameters this would to be
a good time to do some more tidying up:

- ldap ssl ads (we should consider to remove related code)
- unicode (we should consider to remove related code)
- casesignames (alias for case insensitive)
- group (alias for force group)
- only guest (alias for goest only)
- allow hosts (alias for hosts allow)
- deny hosts (alias for hosts deny)
- lock directory (alias for lock dir)
- debuglevel (alias for log level)
- directory (alias for path)
- exec (alias for preexec)
- prefered master (typo alias for preferred master)
- printer (alias for printer name)
- printcap (alias for printcap name)
- private directory (alias for private dir)
- root / root dir ( alias for root directory)
- max protocol (confusing alias for server max protocol)
- protocol (even more confusing parameter for server max protocol)
- min protocol (confusing alias for server min protocol)
- vfs object (alias for vfs objects)
- write ok (rarely used alias for read only / writable)

Björn
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
On Mon, 11 Dec 2017 12:20:38 +0100
Björn JACKE via samba-technical <[hidden email]> wrote:

> Hi,
>
> I think it's not useful to have a huge number of aliases just to make
> the parameters expressable is different humand wordings or spellings.
> They increase the code and binary size and more importantly they make
> config parsing and more difficult and error prone. For some parameter
> aliases which are not very commonly used imho, I'd like to propose to
> remove them for the next release. Given that Metze already removed a
> number of other parameters this would to be a good time to do some
> more tidying up:
>
> - ldap ssl ads (we should consider to remove related code)
> - unicode (we should consider to remove related code)
> - casesignames (alias for case insensitive)
> - group (alias for force group)
> - only guest (alias for goest only)
> - allow hosts (alias for hosts allow)
> - deny hosts (alias for hosts deny)
> - lock directory (alias for lock dir)
> - debuglevel (alias for log level)
> - directory (alias for path)
> - exec (alias for preexec)
> - prefered master (typo alias for preferred master)
> - printer (alias for printer name)
> - printcap (alias for printcap name)
> - private directory (alias for private dir)
> - root / root dir ( alias for root directory)
> - max protocol (confusing alias for server max protocol)
> - protocol (even more confusing parameter for server max protocol)
> - min protocol (confusing alias for server min protocol)
> - vfs object (alias for vfs objects)
> - write ok (rarely used alias for read only / writable)
>
> Björn

Hi Bjorn,
I have never really understood all these synonyms. If the parameter
was created in the first place with a typo, surely this should have
been noticed and fixed before it got into the code, but it seems they
weren't and instead of fixing them correctly, the synonyms were
created. I personally would like to see all the synonyms go away, but
you missed one, writable <-> writeable and this raises the question,
why does samba have 'read only' and 'writeable' ? I have seen numerous
smb.conf files that contain:

read only = No
writeable = Yes

So it seems that a lot of users don't understand they mean the same
thing. Perhaps we should remove 'writable' and 'writeable' as well.

Just my 2p's worth

Rowland
 

Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
On 2017-12-11 at 11:58 +0000 Rowland Penny via samba-technical sent off:
> read only = No
> writeable = Yes

personally I would like this alias also to disappear but I was afraid to
propose something revolutionary like this. Maybe for this one we should
deprecate it first and give it a grace time of one or two leases. For the other
parameters I think this is not required becasue they are rarely used compared
to this one.

Björn

Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Mon, 11 Dec 2017 12:20:38 +0100, Björn JACKE via samba-technical wrote:

> I think it's not useful to have a huge number of aliases just to make the
> parameters expressable is different humand wordings or spellings. They increase
> the code and binary size and more importantly they make config parsing and more
> difficult and error prone. For some parameter aliases which are not very
> commonly used imho, I'd like to propose to remove them for the next release.

I'd be happy to see most for the aliases go, but would prefer to have
them deprecated (with warnings + documentation) for a release first,
rather than directly drop them in the next.

Cheers, David

Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
On Mon, Dec 11, 2017 at 01:27:29PM +0100, David Disseldorp via samba-technical wrote:

> On Mon, 11 Dec 2017 12:20:38 +0100, Björn JACKE via samba-technical wrote:
>
> > I think it's not useful to have a huge number of aliases just to make the
> > parameters expressable is different humand wordings or spellings. They increase
> > the code and binary size and more importantly they make config parsing and more
> > difficult and error prone. For some parameter aliases which are not very
> > commonly used imho, I'd like to propose to remove them for the next release.
>
> I'd be happy to see most for the aliases go, but would prefer to have
> them deprecated (with warnings + documentation) for a release first,
> rather than directly drop them in the next.

+1 from me. It would be better to deprecate them first, then
remove on next release. But yeah, they really should go :-).

Thanks Björn !

Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
On Mon, 2017-12-11 at 09:37 -0800, Jeremy Allison via samba-technical
wrote:

> On Mon, Dec 11, 2017 at 01:27:29PM +0100, David Disseldorp via samba-technical wrote:
> > On Mon, 11 Dec 2017 12:20:38 +0100, Björn JACKE via samba-technical wrote:
> >
> > > I think it's not useful to have a huge number of aliases just to make the
> > > parameters expressable is different humand wordings or spellings. They increase
> > > the code and binary size and more importantly they make config parsing and more
> > > difficult and error prone. For some parameter aliases which are not very
> > > commonly used imho, I'd like to propose to remove them for the next release.
> >
> > I'd be happy to see most for the aliases go, but would prefer to have
> > them deprecated (with warnings + documentation) for a release first,
> > rather than directly drop them in the next.
>
> +1 from me. It would be better to deprecate them first, then
> remove on next release. But yeah, they really should go :-).

I think the deprecation period should depend on the implicatons of the
removal.  We know a lot of folks don't upgrade except every 4 releases
or so, when their distribution is upgraded, particularly Debian stable
or Ubuntu LTS (as these allow in-place upgrades).  They also never read
WHATSNEW.txt

So, for example, removing 'debuglevel' has less implications than 'only
guest' in terms of security.

I'm not saying our new rule is 4 releases notice! But where the only
cost is an alias, consider the cost/benefit.

Thanks,

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


Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On 2017-12-11 at 12:20 +0100 Björn JACKE sent off:
> - unicode (we should consider to remove related code)

patch to deprecate this one is attached

Björn

deprecate-unicode.patch (950 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On 2017-12-11 at 12:20 +0100 Björn JACKE sent off:
> - ldap ssl ads (we should consider to remove related code)

deprecation patch attached.

Björn
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

deprecate-ldapsslads.patch (929 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
Hi,

this patch finally marks the alias parameters as deprecated. Unfortunately
those have to be matched manually. Also the change for the man page will need a
lot mor manual work. I will send a patch for that later. I send this already now
because I hope that the change will make it into master before rc1 is splitted
off.

review and commit to master would be very welcome.

Björn

deprecate-some-alias-parameters.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
Hi Björn,

> this patch finally marks the alias parameters as deprecated. Unfortunately
> those have to be matched manually. Also the change for the man page will need a
> lot mor manual work. I will send a patch for that later. I send this already now
> because I hope that the change will make it into master before rc1 is splitted
> off.
>
> review and commit to master would be very welcome.

I think we should use FLAG_SYNONYM similar to FLAG_DEPRECATED
instead of having a large if statement with explicit checks.

metze



signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
On 2018-01-10 at 17:25 +0100 Stefan Metzmacher via samba-technical sent off:

> > this patch finally marks the alias parameters as deprecated. Unfortunately
> > those have to be matched manually. Also the change for the man page will need a
> > lot mor manual work. I will send a patch for that later. I send this already now
> > because I hope that the change will make it into master before rc1 is splitted
> > off.
> >
> > review and commit to master would be very welcome.
>
> I think we should use FLAG_SYNONYM similar to FLAG_DEPRECATED
> instead of having a large if statement with explicit checks.
exactly that is not easiliy possible unless I don't understand what you mean
exactly here. Deprecating parameters is done via the xml files, a very
soffisticated mechanism - very neat but very difficult to understand and modify
the underlying code (at least for me). If you want to do the deprecation of the
alias parameters in a similar soffisticated way, it would be good if you would
propose a patch for that :-)

Björn

signature.asc (188 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
Am 10.01.2018 um 17:53 schrieb Björn JACKE:

> On 2018-01-10 at 17:25 +0100 Stefan Metzmacher via samba-technical sent off:
>>> this patch finally marks the alias parameters as deprecated. Unfortunately
>>> those have to be matched manually. Also the change for the man page will need a
>>> lot mor manual work. I will send a patch for that later. I send this already now
>>> because I hope that the change will make it into master before rc1 is splitted
>>> off.
>>>
>>> review and commit to master would be very welcome.
>>
>> I think we should use FLAG_SYNONYM similar to FLAG_DEPRECATED
>> instead of having a large if statement with explicit checks.
>
> exactly that is not easiliy possible unless I don't understand what you mean
> exactly here. Deprecating parameters is done via the xml files, a very
> soffisticated mechanism - very neat but very difficult to understand and modify
> the underlying code (at least for me). If you want to do the deprecation of the
> alias parameters in a similar soffisticated way, it would be good if you would
> propose a patch for that :-)
Just something like this:

diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index a18407d..4fbac7a 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -1823,6 +1823,11 @@ bool lpcfg_do_global_parameter(struct
loadparm_context *lp_ctx,
                          pszParmName));
        }

+       if (parm_table[parmnum].flags & FLAG_SYNONYM) {
+               DEBUG(1, ("WARNING: The \"%s\" alias option is
deprecated\n",
+                         pszParmName));
+       }
+
        parm_ptr = lpcfg_parm_ptr(lp_ctx, NULL, &parm_table[parmnum]);

        return set_variable(lp_ctx->globals->ctx, NULL, parmnum, parm_ptr,


bin/default/lib/param/param_table_gen.c seems to set
FLAG_SYNONYM correct.

metze



signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Wed, 10 Jan 2018 17:53:57 +0100
Björn JACKE via samba-technical <[hidden email]> wrote:

> On 2018-01-10 at 17:25 +0100 Stefan Metzmacher via samba-technical
> sent off:
> > > this patch finally marks the alias parameters as deprecated.
> > > Unfortunately those have to be matched manually. Also the change
> > > for the man page will need a lot mor manual work. I will send a
> > > patch for that later. I send this already now because I hope that
> > > the change will make it into master before rc1 is splitted off.
> > >
> > > review and commit to master would be very welcome.
> >
> > I think we should use FLAG_SYNONYM similar to FLAG_DEPRECATED
> > instead of having a large if statement with explicit checks.
>
> exactly that is not easiliy possible unless I don't understand what
> you mean exactly here. Deprecating parameters is done via the xml
> files, a very soffisticated mechanism - very neat but very difficult
> to understand and modify the underlying code (at least for me). If
> you want to do the deprecation of the alias parameters in a similar
> soffisticated way, it would be good if you would propose a patch for
> that :-)
>
> Björn

Hmm, I think I can see why we have all the aliases now LOL

soffisticated != sophisticated ;-)

I personally don't care how they are deprecated, just so long as they
are deprecated.

Rowland


Reply | Threaded
Open this post in threaded view
|

Re: RFC: reduce number of alias parameters

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On 2018-01-10 at 17:57 +0100 Stefan Metzmacher via samba-technical sent off:

> Just something like this:
>
> diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
> index a18407d..4fbac7a 100644
> --- a/lib/param/loadparm.c
> +++ b/lib/param/loadparm.c
> @@ -1823,6 +1823,11 @@ bool lpcfg_do_global_parameter(struct
> loadparm_context *lp_ctx,
>                           pszParmName));
>         }
>
> +       if (parm_table[parmnum].flags & FLAG_SYNONYM) {
> +               DEBUG(1, ("WARNING: The \"%s\" alias option is
> deprecated\n",
> +                         pszParmName));
> +       }
> +
>         parm_ptr = lpcfg_parm_ptr(lp_ctx, NULL, &parm_table[parmnum]);
>
>         return set_variable(lp_ctx->globals->ctx, NULL, parmnum, parm_ptr,
ah, this is the confusion. The intention was not to deprecate *all* alias
parameters, only those dedicated ones listed in the if statements. And
deprecating aliases is not possible without doing it manually currently.
Deprecating all aliases was at least not what I proposed initially. Anyway, I
would also agree going that way and getting rid of all alias parameters for
the upcoming 4.8 release.

Björn

signature.asc (188 bytes) Download Attachment