[PATCH] net conf: add parameter value check to 'net conf setparm'
Michael Adam
obnox at samba.org
Wed Nov 23 11:52:13 UTC 2016
On 2016-11-23 at 12:24 +0100, Ralph Böhme wrote:
> Howdy,
>
> On Wed, Nov 23, 2016 at 12:15:12PM +0100, Ralph Böhme wrote:
> > > this could be combined with your patchset so as to avoid doing the
> > > back and forth business. Attached second patchset demonstrates how
> > > this could look like.
> >
> > Looking into this one...
>
> code lgtm, but I'd like to see an update of the function comment as
> part of the last commit as well please. :)
>
> With an updated comment: rb:me.
Did an update.
Updated patchset attached.
Added your review except for the last patch with the comment.
Also kept Volker's review for the first patch which was not
changed at all.
Thanks - Michael
-------------- next part --------------
From f42890ebaa1ac699b5ba95687152cdd30452bc06 Mon Sep 17 00:00:00 2001
From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date: Tue, 22 Nov 2016 11:20:22 +0100
Subject: [PATCH 1/4] net conf: fix error message
Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Reviewed-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
source3/utils/net_conf_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/utils/net_conf_util.c b/source3/utils/net_conf_util.c
index a188097..ec6a479 100644
--- a/source3/utils/net_conf_util.c
+++ b/source3/utils/net_conf_util.c
@@ -61,7 +61,7 @@ bool net_conf_param_valid(const char *service,
* So the value must be invalid.
*/
d_fprintf(stderr, "invalid value '%s' given for "
- "parameter '%s'\n", param, valstr);
+ "parameter '%s'\n", valstr, param);
return false;
}
--
2.7.4
From 7d42e795814d02fcc030ce837bc9906912b01f55 Mon Sep 17 00:00:00 2001
From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date: Mon, 21 Nov 2016 14:56:52 +0100
Subject: [PATCH 2/4] param: add lp_parameter_value_is_valid() function
Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Reviewed-by: Michael Adam <obnox at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/param/loadparm.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 3e1a15e..b5b9190 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -1062,6 +1062,7 @@ static bool hash_a_service(const char *name, int number);
static void free_service_byindex(int iService);
static void show_parameter(int parmIndex);
static bool is_synonym_of(int parm1, int parm2, bool *inverse);
+static bool lp_parameter_value_is_valid(const char *parm_name, const char *val);
/*
* This is a helper function for parametrical options support. It returns a
@@ -1852,6 +1853,71 @@ static void show_parameter(int parmIndex)
printf("\n");
}
+/*
+ * Check the value for a P_ENUM
+ */
+static bool check_enum_parameter(struct parm_struct *parm, const char *value)
+{
+ int i;
+
+ for (i = 0; parm->enum_list[i].name; i++) {
+ if (strwicmp(value, parm->enum_list[i].name) == 0) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/**************************************************************************
+ Check whether the given value is valid for the given parameter name.
+**************************************************************************/
+
+static bool lp_parameter_value_is_valid(const char *parm_name, const char *val)
+{
+ bool ret = false, tmp_bool;
+ int num = lpcfg_map_parameter(parm_name), tmp_int;
+ uint64_t tmp_int64 = 0;
+ struct parm_struct *parm;
+
+ if (num >= 0) {
+ parm = &parm_table[num];
+ switch (parm->type) {
+ case P_BOOL:
+ case P_BOOLREV:
+ ret = set_boolean(val, &tmp_bool);
+ break;
+
+ case P_INTEGER:
+ ret = (sscanf(val, "%d", &tmp_int) == 1);
+ break;
+
+ case P_OCTAL:
+ ret = (sscanf(val, "%o", &tmp_int) == 1);
+ break;
+
+ case P_ENUM:
+ ret = check_enum_parameter(parm, val);
+ break;
+
+ case P_BYTES:
+ if (conv_str_size_error(val, &tmp_int64) &&
+ tmp_int64 <= INT_MAX) {
+ ret = true;
+ }
+ break;
+
+ case P_CHAR:
+ case P_LIST:
+ case P_STRING:
+ case P_USTRING:
+ case P_CMDLIST:
+ ret = true;
+ break;
+ }
+ }
+ return ret;
+}
+
/***************************************************************************
Show all parameter's name, type, [values,] and flags.
***************************************************************************/
--
2.7.4
From 6a7efe3f44c85b07ebf769e2a48c4d7a03320b09 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 23 Nov 2016 11:12:42 +0100
Subject: [PATCH 3/4] param: use early return in
lp_canonicalize_parameter_with_value()
This reduces the indentation and streamlines the flow.
View with "git show -w" to see it's mostly indentation change.
Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/param/loadparm.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index b5b9190..6ac7b69 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -1713,16 +1713,17 @@ bool lp_canonicalize_parameter_with_value(const char *parm_name,
/* parametric option */
*canon_parm = parm_name;
*canon_val = val;
- } else {
- *canon_parm = parm_table[num].label;
- if (inverse) {
- if (!lp_invert_boolean(val, canon_val)) {
- *canon_val = NULL;
- return false;
- }
- } else {
- *canon_val = val;
+ return true;
+ }
+
+ *canon_parm = parm_table[num].label;
+ if (inverse) {
+ if (!lp_invert_boolean(val, canon_val)) {
+ *canon_val = NULL;
+ return false;
}
+ } else {
+ *canon_val = val;
}
return true;
--
2.7.4
From 6319777c5be928d4e64a44faab0e772279b53fab Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 23 Nov 2016 11:14:54 +0100
Subject: [PATCH 4/4] param: validate value in
lp_canonicalize_parameter_with_value()
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/param/loadparm.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 6ac7b69..21073c6 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -1688,9 +1688,11 @@ bool lp_canonicalize_parameter(const char *parm_name, const char **canon_parm,
Turn the value given into the inverse boolean expression when
the synonym is an invers boolean synonym.
- Return true if parm_name is a valid parameter name and
- in case it is an invers boolean synonym, if the val string could
- successfully be converted to the reverse bool.
+ Return true if
+ - parm_name is a valid parameter name and
+ - val is a valid value for this parameter and
+ - in case the parameter is an inverse boolean synonym, if the val
+ string could successfully be converted to the reverse bool.
Return false in all other cases.
**************************************************************************/
@@ -1701,6 +1703,7 @@ bool lp_canonicalize_parameter_with_value(const char *parm_name,
{
int num;
bool inverse;
+ bool ret;
if (!lp_parameter_is_valid(parm_name)) {
*canon_parm = NULL;
@@ -1726,7 +1729,9 @@ bool lp_canonicalize_parameter_with_value(const char *parm_name,
*canon_val = val;
}
- return true;
+ ret = lp_parameter_value_is_valid(*canon_parm, *canon_val);
+
+ return ret;
}
/***************************************************************************
--
2.7.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161123/f1975e56/signature.sig>
More information about the samba-technical
mailing list