[PATCH] Improve samba-tool testparm and remove unused script
Jelmer Vernooij
jelmer at samba.org
Fri Jan 24 12:30:35 MST 2014
On Sat, Jan 25, 2014 at 08:17:48AM +1300, Andrew Bartlett wrote:
> On Fri, 2014-01-24 at 13:28 +0100, Jelmer Vernooij wrote:
> > On Fri, Jan 24, 2014 at 09:06:23PM +1300, Andrew Bartlett wrote:
> > > From a2b0fbde9d4118bc8bc14f410511436bda83fa3a Mon Sep 17 00:00:00 2001
> > > From: Andrew Bartlett <abartlet at samba.org>
> > > Date: Tue, 24 Dec 2013 14:06:47 +1300
> > > Subject: [PATCH 1/4] script: Remove unused and no-longer-working
> > > extract_allparms.sh
> > >
> > > Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> > > Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> > Reviewed-By: Jelmer Vernooij <jelmer at samba.org>
> >
> > > From ff6495e4683d7cecf4fe289c91d930e2c6f5b5ab Mon Sep 17 00:00:00 2001
> > > From: Garming Sam <garming at catalyst.net.nz>
> > > Date: Fri, 27 Dec 2013 17:09:35 +1300
> > > Subject: [PATCH 2/4] testparm: fix --parameter-name failure to convert to
> > > output string
> >
> > > Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> > > Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> > > ---
> > > python/samba/netcmd/testparm.py | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/python/samba/netcmd/testparm.py b/python/samba/netcmd/testparm.py
> > > index 9251469..dbf79c7 100644
> > > --- a/python/samba/netcmd/testparm.py
> > > +++ b/python/samba/netcmd/testparm.py
> > > @@ -105,7 +105,7 @@ class cmd_testparm(Command):
> > > lp[section_name].dump(sys.stdout, lp.default_service,
> > > verbose)
> > > else:
> > > - self.outf.write(lp.get(parameter_name, section_name)+"\n")
> > > + self.outf.write(str(lp.get(parameter_name, section_name))+"\n")
> >
> > What this is trying to avoid? Doesn't sys.stdout.write already cast to string?
>
> Correct, we get:
>
> ERROR(<type 'exceptions.TypeError'>): uncaught exception - can only concatenate list (not "str") to list
> File "bin/python/samba/netcmd/__init__.py", line 175, in _run
> return self.run(*args, **kwargs)
> File "bin/python/samba/netcmd/testparm.py", line 108, in run
> self.outf.write(lp.get(parameter_name, section_name)+"\n")
Do we really want to simply call str in that case though? That will result in
output that is fairly python specific, rather than similar to smb.conf syntax:
>>> x = ["a", "b", "c"]
>>> str(x)
"['a', 'b', 'c']"
A test to verify this behaviour would also be nice. :-)
> > > From 8c58c792f1ea89f72eec8013dbe7d8cc5802ed3c Mon Sep 17 00:00:00 2001
> > > From: Garming Sam <garming at catalyst.net.nz>
> > > Date: Tue, 31 Dec 2013 11:56:27 +1300
> > > Subject: [PATCH 3/4] s4-testparm: modify dumping of parameters to use the
> > > lib/param code to be more consistent
> >
> > More consistent in what regard? Has the output changed?
>
> Yes, this (perhaps it needs to be squashed with the previous one)
> changes the output to match the code used for the main dump routine.
>
> Otherwise, we get:
> [abartlet at jesse samba]$ bin/samba-tool testparm --suppress-prompt
> --parameter-name="directory mask"
> 493
>
> We want instead:
> bin/samba-tool testparm --suppress-prompt --parameter-name="directory
> mask"
> 0755
Okay.
> > > Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> > > Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> > > ---
> > > python/samba/netcmd/testparm.py | 2 +-
> > > source4/param/pyparam.c | 38 ++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 39 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/python/samba/netcmd/testparm.py b/python/samba/netcmd/testparm.py
> > > index dbf79c7..7825d25 100644
> > > --- a/python/samba/netcmd/testparm.py
> > > +++ b/python/samba/netcmd/testparm.py
> > > @@ -105,7 +105,7 @@ class cmd_testparm(Command):
> > > lp[section_name].dump(sys.stdout, lp.default_service,
> > > verbose)
> > > else:
> > > - self.outf.write(str(lp.get(parameter_name, section_name))+"\n")
> > > + lp.dump_a_parameter(sys.stdout, parameter_name, section_name)
> > > else:
> > > if not suppress_prompt:
> > > self.outf.write("Press enter to see a dump of your service definitions\n")
> > > diff --git a/source4/param/pyparam.c b/source4/param/pyparam.c
> > > index 9874006..0fde10f 100644
> > > --- a/source4/param/pyparam.c
> > > +++ b/source4/param/pyparam.c
> > > @@ -288,6 +288,42 @@ static PyObject *py_lp_dump(PyObject *self, PyObject *args)
> > > Py_RETURN_NONE;
> > > }
> > >
> > > +static PyObject *py_lp_dump_a_parameter(PyObject *self, PyObject *args)
> > > +{
> > > + PyObject *py_stream;
> > > + char *param_name;
> > > + char *section_name = NULL;
> > > + FILE *f;
> > > + struct loadparm_context *lp_ctx = PyLoadparmContext_AsLoadparmContext(self);
> > > + struct loadparm_service *service;
> > > +
> > > + if (!PyArg_ParseTuple(args, "Os|z", &py_stream, ¶m_name, §ion_name))
> > > + return NULL;
> > > +
> > > + f = PyFile_AsFile(py_stream);
> > > + if (f == NULL) {
> > > + PyErr_SetString(PyExc_TypeError, "Not a file stream");
> > ^^ Doesn't PyFile_AsFile set a PyErr already?
>
> It's just a copy of what was added in:
>
> commit de3f9e31d34eac6ddc17e298299d5065f9a86e7c
> Author: Jelmer Vernooij <jelmer at samba.org>
> Date: Sun Jun 20 13:40:49 2010 +0200
>
> s4-python: Add LoadparmService.dump()
I think the extra call to PyErr_SetString() is incorrect too. There could be
other reasons why PyFile_AsFile() fails. Sorry for setting a bad example :-(
> > > + return NULL;
> > > + }
> > > +
> > > + if (section_name != NULL && strwicmp(section_name, GLOBAL_NAME) &&
> > > + strwicmp(section_name, GLOBAL_NAME2)) {
> > > + /* it's a share parameter */
> > > + service = lpcfg_service(lp_ctx, section_name);
> > > + if (service == NULL) {
> > > + Py_RETURN_NONE;
> > > + }
> > > + } else {
> > > + /* it's global */
> > > + service = NULL;
> > > + }
> > > +
> > > + lpcfg_dump_a_parameter(lp_ctx, service, param_name, f);
> > > +
> > > + Py_RETURN_NONE;
> > > +
> > > +}
> > > +
> > > static PyObject *py_samdb_url(PyObject *self)
> > > {
> > > struct loadparm_context *lp_ctx = PyLoadparmContext_AsLoadparmContext(self);
> > > @@ -323,6 +359,8 @@ static PyMethodDef py_lp_ctx_methods[] = {
> > > "Get the server role." },
> > > { "dump", (PyCFunction)py_lp_dump, METH_VARARGS,
> > > "S.dump(stream, show_defaults=False)" },
> > > + { "dump_a_parameter", (PyCFunction)py_lp_dump_a_parameter, METH_VARARGS,
> > > + "S.dump_a_parameter(stream, name, service_name)" },
> > > { "samdb_url", (PyCFunction)py_samdb_url, METH_NOARGS,
> > > "S.samdb_url() -> string\n"
> > > "Returns the current URL for sam.ldb." },
> > > --
> > > 1.8.4.2
> > >
>
> Thanks Jelmer. It looks like "[PATCH 4/4] lib/param: fix parameter
> dumping to detect share and global parameters" has a regression on
> running 'bin/samba-tool testparm --suppress-prompt
> --parameter-name="directory mask"' without also specifying a
> --section-name, so Garming and I will to look into that some more, and
> add a unit test.
Thanks.
Cheers,
jelmer
More information about the samba-technical
mailing list