where should smb.conf % macros be supported?
Michael Adam
obnox at samba.org
Tue May 31 05:41:22 MDT 2011
Hi Andrew,
I would very much appreciate if this change could be done
(as Volker has already suggested) one parameter per commit.
I think it is dangerous to attempt to change too many things
in one commit. It will make debugging more difficult if
something breaks.
Other than that, I agree to making these parameters const!
Cheers - Michael
Andrew Bartlett wrote:
> On Wed, 2011-05-25 at 08:21 +1000, Andrew Bartlett wrote:
> > On Tue, 2011-05-24 at 17:32 +0200, Volker Lendecke wrote:
> > > Hi, Andrew!
> > >
> > > I've been there, many times. I know lp_string is one of the
> > > dirtiest pieces that Samba has, and probably the most
> > > difficult one to extinguish.
> > >
> > > The approach I had tried once was to make all the expansion
> > > variables explicit. Maybe we should pick the ones that have
> > > only very few callers, convert those to CONST_STRING and do
> > > the required expansion explicitly in the callers. This is
> > > really a case-by-case conversion unfortunately, at least
> > > that's my 2ct on the issue.
> >
> > Thank you very much for the review. The background is very helpful in
> > trying to pin down a set of semantics here.
> >
> > > On Tue, May 24, 2011 at 12:02:12PM +1000, Andrew Bartlett wrote:
> > > > To start bridging the gap, I would like to convert the following
> > > > parameters to FN_GLOBAL_CONST_STRING, which are truly constant (at least
> > > > until an smb.conf reload):
> > > >
> > > > FN_GLOBAL_STRING(lp_dos_charset, &Globals.dos_charset)
> > > > FN_GLOBAL_STRING(lp_unix_charset, &Globals.unix_charset)
> > > > FN_GLOBAL_STRING(lp_display_charset, &Globals.display_charset)
> > >
> > > > FN_GLOBAL_STRING(lp_logfile, &Globals.szLogFile)
> > > > FN_GLOBAL_STRING(lp_configfile, &Globals.szConfigFile)
> > >
> > > Both log file and config file definitely need the
> > > expansions.
> >
> > Indeed (they, and 'include =' are the items that perhaps will always
> > need the full range of expansions).
> >
> > > > FN_GLOBAL_STRING(lp_smb_passwd_file, &Globals.szSMBPasswdFile)
> > > > FN_GLOBAL_STRING(lp_private_dir, &Globals.szPrivateDir)
> > > > FN_GLOBAL_STRING(lp_lockdir, &Globals.szLockDir)
> > > > FN_GLOBAL_STRING(lp_piddir, &Globals.szPidDir)
> > > > FN_GLOBAL_STRING(lp_utmpdir, &Globals.szUtmpDir)
> > > > FN_GLOBAL_STRING(lp_wtmpdir, &Globals.szWtmpDir)
> > > > FN_GLOBAL_STRING(lp_realm, &Globals.szRealm)
> > > > FN_GLOBAL_STRING(lp_name_resolve_order, &Globals.szNameResolveOrder)
> > > > FN_GLOBAL_STRING(lp_panic_action, &Globals.szPanicAction)
> > > > FN_GLOBAL_STRING(lp_dedicated_keytab_file,
> > > > &Globals.szDedicatedKeytabFile)
> > > > FN_GLOBAL_STRING(lp_ncalrpc_dir, &Globals.ncalrpc_dir)
> > > > FN_GLOBAL_STRING(lp_passwordserver, &Globals.szPasswordServer)
> > >
> > > The other ones I could see as CONST_STRING.
> >
> > Thanks, it is this list (with the addition of passdb backend, smb ports)
> > that I need most urgently to be in common and would like to become
> > CONST_STRING, without any subs.
> >
> > My remaining question is about ensuring I do this right by our users:
> > For this set, do we need to warn our users with a deprecation message in
> > 3.6.0?
>
> Attached is the patch I propose for master, which should cover enough of
> the basic parameters for me to start on a s3_loadparm_ctx()
> implementation.
>
> Andrew Bartlett
>
> --
> Andrew Bartlett http://samba.org/~abartlet/
> Authentication Developer, Samba Team http://samba.org
> >From a39307f327dd5e730a1faaa8febbbf04cd240189 Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Tue, 31 May 2011 18:11:28 +1000
> Subject: [PATCH] s3-param Make a number of parameters const
>
> These core parameters should not change due to % macro expansions, and
> so are now fixed as constant.
>
> The documentation for 'password server' is amended to remove reference
> to this no longer supported feature (security=server, which enabled
> this mode, has already been marked as depricated in 3.6).
>
> Andrew Bartlett
> ---
> docs-xml/smbdotconf/security/passwordserver.xml | 8 ----
> source3/include/proto.h | 36 ++++++++--------
> source3/libads/ldap.c | 2 +-
> source3/param/loadparm.c | 52 +++++++++++-----------
> source3/smbd/server.c | 2 +-
> 5 files changed, 46 insertions(+), 54 deletions(-)
>
> diff --git a/docs-xml/smbdotconf/security/passwordserver.xml b/docs-xml/smbdotconf/security/passwordserver.xml
> index 0ac39f1..09d335c 100644
> --- a/docs-xml/smbdotconf/security/passwordserver.xml
> +++ b/docs-xml/smbdotconf/security/passwordserver.xml
> @@ -82,14 +82,6 @@
> This will cause a loop and could lock up your Samba server!</para>
> </listitem>
>
> - <listitem>
> - <para>The name of the password server takes the standard
> - substitutions, but probably the only useful one is <parameter moreinfo="none">%m
> - </parameter>, which means the Samba server will use the incoming
> - client as the password server. If you use this then you better
> - trust your clients, and you had better restrict them with hosts allow!</para>
> - </listitem>
> -
> </itemizedlist>
> </description>
>
> diff --git a/source3/include/proto.h b/source3/include/proto.h
> index 23654e1..46b46d7 100644
> --- a/source3/include/proto.h
> +++ b/source3/include/proto.h
> @@ -1175,14 +1175,14 @@ NTSTATUS change_trust_account_password( const char *domain, const char *remote_m
>
> /* The following definitions come from param/loadparm.c */
>
> -char *lp_smb_ports(void);
> -char *lp_dos_charset(void);
> -char *lp_unix_charset(void);
> -char *lp_display_charset(void);
> +const char *lp_smb_ports(void);
> +const char *lp_dos_charset(void);
> +const char *lp_unix_charset(void);
> +const char *lp_display_charset(void);
> char *lp_logfile(void);
> char *lp_configfile(void);
> -char *lp_smb_passwd_file(void);
> -char *lp_private_dir(void);
> +const char *lp_smb_passwd_file(void);
> +const char *lp_private_dir(void);
> char *lp_serverstring(void);
> int lp_printcap_cache_time(void);
> char *lp_addport_cmd(void);
> @@ -1190,14 +1190,14 @@ char *lp_enumports_cmd(void);
> char *lp_addprinter_cmd(void);
> char *lp_deleteprinter_cmd(void);
> char *lp_os2_driver_map(void);
> -char *lp_lockdir(void);
> -char *lp_statedir(void);
> -char *lp_cachedir(void);
> -char *lp_piddir(void);
> +const char *lp_lockdir(void);
> +const char *lp_statedir(void);
> +const char *lp_cachedir(void);
> +const char *lp_piddir(void);
> char *lp_mangling_method(void);
> int lp_mangle_prefix(void);
> -char *lp_utmpdir(void);
> -char *lp_wtmpdir(void);
> +const char *lp_utmpdir(void);
> +const char *lp_wtmpdir(void);
> bool lp_utmp(void);
> char *lp_rootdir(void);
> char *lp_defaultservice(void);
> @@ -1207,9 +1207,9 @@ char *lp_set_quota_command(void);
> char *lp_auto_services(void);
> char *lp_passwd_program(void);
> char *lp_passwd_chat(void);
> -char *lp_passwordserver(void);
> -char *lp_name_resolve_order(void);
> -char *lp_realm(void);
> +const char *lp_passwordserver(void);
> +const char *lp_name_resolve_order(void);
> +const char *lp_realm(void);
> const char *lp_afs_username_map(void);
> int lp_afs_token_lifetime(void);
> char *lp_log_nt_token_command(void);
> @@ -1228,7 +1228,7 @@ char *lp_nis_home_map_name(void);
> const char **lp_netbios_aliases(void);
> const char *lp_passdb_backend(void);
> const char **lp_preload_modules(void);
> -char *lp_panic_action(void);
> +const char *lp_panic_action(void);
> char *lp_adduser_script(void);
> char *lp_renameuser_script(void);
> char *lp_deluser_script(void);
> @@ -1352,7 +1352,7 @@ bool lp_send_spnego_principal(void);
> bool lp_hostname_lookups(void);
> bool lp_change_notify(const struct share_params *p );
> bool lp_kernel_change_notify(const struct share_params *p );
> -char * lp_dedicated_keytab_file(void);
> +const char * lp_dedicated_keytab_file(void);
> int lp_kerberos_method(void);
> bool lp_defer_sharing_violations(void);
> bool lp_enable_privileges(void);
> @@ -1660,7 +1660,7 @@ int lp_min_receive_file_size(void);
> char* lp_perfcount_module(void);
> void lp_set_passdb_backend(const char *backend);
> void widelinks_warning(int snum);
> -char *lp_ncalrpc_dir(void);
> +const char *lp_ncalrpc_dir(void);
>
> /* The following definitions come from param/loadparm_server_role.c */
>
> diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
> index 19cb3ad..375dc3b 100644
> --- a/source3/libads/ldap.c
> +++ b/source3/libads/ldap.c
> @@ -529,7 +529,7 @@ ADS_STATUS ads_connect_gc(ADS_STRUCT *ads)
> TALLOC_CTX *frame = talloc_stackframe();
> struct dns_rr_srv *gcs_list;
> int num_gcs;
> - char *realm = ads->server.realm;
> + const char *realm = ads->server.realm;
> NTSTATUS nt_status = NT_STATUS_UNSUCCESSFUL;
> ADS_STATUS ads_status = ADS_ERROR_NT(NT_STATUS_UNSUCCESSFUL);
> int i;
> diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
> index 08597ad..817e79d 100644
> --- a/source3/param/loadparm.c
> +++ b/source3/param/loadparm.c
> @@ -5564,14 +5564,14 @@ static char *lp_string(const char *s)
> #define FN_LOCAL_CHAR(fn_name,val) \
> char fn_name(const struct share_params *p) {return(LP_SNUM_OK(p->service)? ServicePtrs[(p->service)]->val : sDefault.val);}
>
> -FN_GLOBAL_STRING(lp_smb_ports, &Globals.smb_ports)
> -FN_GLOBAL_STRING(lp_dos_charset, &Globals.dos_charset)
> -FN_GLOBAL_STRING(lp_unix_charset, &Globals.unix_charset)
> -FN_GLOBAL_STRING(lp_display_charset, &Globals.display_charset)
> +FN_GLOBAL_CONST_STRING(lp_smb_ports, &Globals.smb_ports)
> +FN_GLOBAL_CONST_STRING(lp_dos_charset, &Globals.dos_charset)
> +FN_GLOBAL_CONST_STRING(lp_unix_charset, &Globals.unix_charset)
> +FN_GLOBAL_CONST_STRING(lp_display_charset, &Globals.display_charset)
> FN_GLOBAL_STRING(lp_logfile, &Globals.szLogFile)
> FN_GLOBAL_STRING(lp_configfile, &Globals.szConfigFile)
> -FN_GLOBAL_STRING(lp_smb_passwd_file, &Globals.szSMBPasswdFile)
> -FN_GLOBAL_STRING(lp_private_dir, &Globals.szPrivateDir)
> +FN_GLOBAL_CONST_STRING(lp_smb_passwd_file, &Globals.szSMBPasswdFile)
> +FN_GLOBAL_CONST_STRING(lp_private_dir, &Globals.szPrivateDir)
> FN_GLOBAL_STRING(lp_serverstring, &Globals.szServerString)
> FN_GLOBAL_INTEGER(lp_printcap_cache_time, &Globals.PrintcapCacheTime)
> FN_GLOBAL_STRING(lp_addport_cmd, &Globals.szAddPortCommand)
> @@ -5579,33 +5579,33 @@ FN_GLOBAL_STRING(lp_enumports_cmd, &Globals.szEnumPortsCommand)
> FN_GLOBAL_STRING(lp_addprinter_cmd, &Globals.szAddPrinterCommand)
> FN_GLOBAL_STRING(lp_deleteprinter_cmd, &Globals.szDeletePrinterCommand)
> FN_GLOBAL_STRING(lp_os2_driver_map, &Globals.szOs2DriverMap)
> -FN_GLOBAL_STRING(lp_lockdir, &Globals.szLockDir)
> +FN_GLOBAL_CONST_STRING(lp_lockdir, &Globals.szLockDir)
> /* If lp_statedir() and lp_cachedir() are explicitely set during the
> * build process or in smb.conf, we use that value. Otherwise they
> * default to the value of lp_lockdir(). */
> -char *lp_statedir(void) {
> +const char *lp_statedir(void) {
> if ((strcmp(get_dyn_STATEDIR(), get_dyn_LOCKDIR()) != 0) ||
> (strcmp(get_dyn_STATEDIR(), Globals.szStateDir) != 0))
> - return(lp_string(*(char **)(&Globals.szStateDir) ?
> - *(char **)(&Globals.szStateDir) : ""));
> + return(*(char **)(&Globals.szStateDir) ?
> + *(char **)(&Globals.szStateDir) : "");
> else
> - return(lp_string(*(char **)(&Globals.szLockDir) ?
> - *(char **)(&Globals.szLockDir) : ""));
> + return(*(char **)(&Globals.szLockDir) ?
> + *(char **)(&Globals.szLockDir) : "");
> }
> -char *lp_cachedir(void) {
> +const char *lp_cachedir(void) {
> if ((strcmp(get_dyn_CACHEDIR(), get_dyn_LOCKDIR()) != 0) ||
> (strcmp(get_dyn_CACHEDIR(), Globals.szCacheDir) != 0))
> - return(lp_string(*(char **)(&Globals.szCacheDir) ?
> - *(char **)(&Globals.szCacheDir) : ""));
> + return(*(char **)(&Globals.szCacheDir) ?
> + *(char **)(&Globals.szCacheDir) : "");
> else
> - return(lp_string(*(char **)(&Globals.szLockDir) ?
> - *(char **)(&Globals.szLockDir) : ""));
> + return(*(char **)(&Globals.szLockDir) ?
> + *(char **)(&Globals.szLockDir) : "");
> }
> -FN_GLOBAL_STRING(lp_piddir, &Globals.szPidDir)
> +FN_GLOBAL_CONST_STRING(lp_piddir, &Globals.szPidDir)
> FN_GLOBAL_STRING(lp_mangling_method, &Globals.szManglingMethod)
> FN_GLOBAL_INTEGER(lp_mangle_prefix, &Globals.mangle_prefix)
> -FN_GLOBAL_STRING(lp_utmpdir, &Globals.szUtmpDir)
> -FN_GLOBAL_STRING(lp_wtmpdir, &Globals.szWtmpDir)
> +FN_GLOBAL_CONST_STRING(lp_utmpdir, &Globals.szUtmpDir)
> +FN_GLOBAL_CONST_STRING(lp_wtmpdir, &Globals.szWtmpDir)
> FN_GLOBAL_BOOL(lp_utmp, &Globals.bUtmp)
> FN_GLOBAL_STRING(lp_rootdir, &Globals.szRootdir)
> FN_GLOBAL_STRING(lp_perfcount_module, &Globals.szSMBPerfcountModule)
> @@ -5616,9 +5616,9 @@ FN_GLOBAL_STRING(lp_set_quota_command, &Globals.szSetQuota)
> FN_GLOBAL_STRING(lp_auto_services, &Globals.szAutoServices)
> FN_GLOBAL_STRING(lp_passwd_program, &Globals.szPasswdProgram)
> FN_GLOBAL_STRING(lp_passwd_chat, &Globals.szPasswdChat)
> -FN_GLOBAL_STRING(lp_passwordserver, &Globals.szPasswordServer)
> -FN_GLOBAL_STRING(lp_name_resolve_order, &Globals.szNameResolveOrder)
> -FN_GLOBAL_STRING(lp_realm, &Globals.szRealm)
> +FN_GLOBAL_CONST_STRING(lp_passwordserver, &Globals.szPasswordServer)
> +FN_GLOBAL_CONST_STRING(lp_name_resolve_order, &Globals.szNameResolveOrder)
> +FN_GLOBAL_CONST_STRING(lp_realm, &Globals.szRealm)
> FN_GLOBAL_CONST_STRING(lp_afs_username_map, &Globals.szAfsUsernameMap)
> FN_GLOBAL_INTEGER(lp_afs_token_lifetime, &Globals.iAfsTokenLifetime)
> FN_GLOBAL_STRING(lp_log_nt_token_command, &Globals.szLogNtTokenCommand)
> @@ -5678,7 +5678,7 @@ out:
> return Globals.szPassdbBackend;
> }
> FN_GLOBAL_LIST(lp_preload_modules, &Globals.szPreloadModules)
> -FN_GLOBAL_STRING(lp_panic_action, &Globals.szPanicAction)
> +FN_GLOBAL_CONST_STRING(lp_panic_action, &Globals.szPanicAction)
> FN_GLOBAL_STRING(lp_adduser_script, &Globals.szAddUserScript)
> FN_GLOBAL_STRING(lp_renameuser_script, &Globals.szRenameUserScript)
> FN_GLOBAL_STRING(lp_deluser_script, &Globals.szDelUserScript)
> @@ -5823,7 +5823,7 @@ FN_GLOBAL_BOOL(lp_send_spnego_principal, &Globals.send_spnego_principal)
> FN_GLOBAL_BOOL(lp_hostname_lookups, &Globals.bHostnameLookups)
> FN_LOCAL_PARM_BOOL(lp_change_notify, bChangeNotify)
> FN_LOCAL_PARM_BOOL(lp_kernel_change_notify, bKernelChangeNotify)
> -FN_GLOBAL_STRING(lp_dedicated_keytab_file, &Globals.szDedicatedKeytabFile)
> +FN_GLOBAL_CONST_STRING(lp_dedicated_keytab_file, &Globals.szDedicatedKeytabFile)
> FN_GLOBAL_INTEGER(lp_kerberos_method, &Globals.iKerberosMethod)
> FN_GLOBAL_BOOL(lp_defer_sharing_violations, &Globals.bDeferSharingViolations)
> FN_GLOBAL_BOOL(lp_enable_privileges, &Globals.bEnablePrivileges)
> @@ -6048,7 +6048,7 @@ FN_GLOBAL_INTEGER(lp_client_signing, &Globals.client_signing)
> FN_GLOBAL_INTEGER(lp_server_signing, &Globals.server_signing)
> FN_GLOBAL_INTEGER(lp_client_ldap_sasl_wrapping, &Globals.client_ldap_sasl_wrapping)
>
> -FN_GLOBAL_STRING(lp_ncalrpc_dir, &Globals.ncalrpc_dir)
> +FN_GLOBAL_CONST_STRING(lp_ncalrpc_dir, &Globals.ncalrpc_dir)
>
> /* local prototypes */
>
> diff --git a/source3/smbd/server.c b/source3/smbd/server.c
> index fc6ab3a..5aa3ddb 100644
> --- a/source3/smbd/server.c
> +++ b/source3/smbd/server.c
> @@ -597,7 +597,7 @@ static bool open_sockets_smbd(struct smbd_parent_context *parent,
> {
> int num_interfaces = iface_count();
> int i;
> - char *ports;
> + const char *ports;
> unsigned dns_port = 0;
>
> #ifdef HAVE_ATEXIT
> --
> 1.7.4.4
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20110531/f7819917/attachment.pgp>
More information about the samba-technical
mailing list