[PATCH] Improve samba-tool testparm and remove unused script
Andrew Bartlett
abartlet at samba.org
Fri Jan 24 12:44:46 MST 2014
On Fri, 2014-01-24 at 20:30 +0100, Jelmer Vernooij wrote:
> 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. :-)
The docs.py test is what found and will verify this, because it now (in
the full branch) checks the output against the 'man smb.conf' contents,
including formatting.
> > > > 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 :-(
Can we put this in, and you get me a correct patch to fix both?
> > > > + 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.
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
More information about the samba-technical
mailing list