| Message ID | 20170124174534.3865-2-robh@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dtc unit-address and character set checks | expand |
On Tue, Jan 24, 2017 at 11:45:30AM -0600, Rob Herring wrote: > While '?', '.', '+', '*', and '_' are considered valid characters their > use is discouraged in recommended practices. '#' is also only > recommended to be used at the beginning of property names. Hrm. I'm not totally convinced about this. PAPR definitely specifies a few properties that include '#' not at the beginning. I'm not sure that IEEE1275 doesn't as well (it's not exactly easy to grep for a single character). I'm not sure about '?' either. I didn't positively identify any properties including it, but there are certainly OF configuration variables specified by PAPR including '?', and I think those will be exposed in the device tree as properties. I'm not 100% sure about '*' either, I have a vague memory of some spec that requires it, but I'm not certain. AFAIK, excluding '.', '+' and '_' should be ok, except for device_type which you already have special exception for. > Testing this found one typo error with '.' used instead of ','. The > rest of the warnings were all from underscores. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > checks.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/checks.c b/checks.c > index 3d18e45374c8..a0d4a9d968d7 100644 > --- a/checks.c > +++ b/checks.c > @@ -239,6 +239,7 @@ ERROR(duplicate_property_names, check_duplicate_property_names, NULL); > #define UPPERCASE "ABCDEFGHIJKLMNOPQRSTUVWXYZ" > #define DIGITS "0123456789" > #define PROPNODECHARS LOWERCASE UPPERCASE DIGITS ",._+*#?-" > +#define PROPNODECHARSSTRICT LOWERCASE UPPERCASE DIGITS ",-" > > static void check_node_name_chars(struct check *c, struct dt_info *dti, > struct node *node) > @@ -299,6 +300,38 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti, > } > ERROR(property_name_chars, check_property_name_chars, PROPNODECHARS); > > +static void check_property_name_chars_strict(struct check *c, > + struct dt_info *dti, > + struct node *node) > +{ > + struct property *prop; > + > + for_each_property(node, prop) { > + const char *name = prop->name; > + int n = strspn(name, c->data); > + > + if (n == strlen(prop->name)) > + continue; > + > + /* Certain names are whitelisted */ > + if (strcmp(name, "device_type") == 0) > + continue; > + > + /* > + * # is only allowed at the beginning of property names not counting > + * the vendor prefix. > + */ > + if (name[n] == '#' && ((n == 0) || (name[n-1] == ','))) { > + name += n + 1; > + n = strspn(name, c->data); > + } > + if (n < strlen(name)) > + FAIL(c, "Character '%c' not recommended in property name \"%s\", node %s", > + name[n], prop->name, node->fullpath); > + } > +} > +WARNING(property_name_chars_strict, check_property_name_chars_strict, PROPNODECHARSSTRICT); > + > #define DESCLABEL_FMT "%s%s%s%s%s" > #define DESCLABEL_ARGS(node,prop,mark) \ > ((mark) ? "value of " : ""), \ > @@ -703,6 +736,8 @@ static struct check *check_table[] = { > &address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell, > &device_type_is_string, &model_is_string, &status_is_string, > > + &property_name_chars_strict, > + > &addr_size_cells, ®_format, &ranges_format, > > &unit_address_vs_reg, -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
diff --git a/checks.c b/checks.c index 3d18e45374c8..a0d4a9d968d7 100644 --- a/checks.c +++ b/checks.c @@ -239,6 +239,7 @@ ERROR(duplicate_property_names, check_duplicate_property_names, NULL); #define UPPERCASE "ABCDEFGHIJKLMNOPQRSTUVWXYZ" #define DIGITS "0123456789" #define PROPNODECHARS LOWERCASE UPPERCASE DIGITS ",._+*#?-" +#define PROPNODECHARSSTRICT LOWERCASE UPPERCASE DIGITS ",-" static void check_node_name_chars(struct check *c, struct dt_info *dti, struct node *node) @@ -299,6 +300,38 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti, } ERROR(property_name_chars, check_property_name_chars, PROPNODECHARS); +static void check_property_name_chars_strict(struct check *c, + struct dt_info *dti, + struct node *node) +{ + struct property *prop; + + for_each_property(node, prop) { + const char *name = prop->name; + int n = strspn(name, c->data); + + if (n == strlen(prop->name)) + continue; + + /* Certain names are whitelisted */ + if (strcmp(name, "device_type") == 0) + continue; + + /* + * # is only allowed at the beginning of property names not counting + * the vendor prefix. + */ + if (name[n] == '#' && ((n == 0) || (name[n-1] == ','))) { + name += n + 1; + n = strspn(name, c->data); + } + if (n < strlen(name)) + FAIL(c, "Character '%c' not recommended in property name \"%s\", node %s", + name[n], prop->name, node->fullpath); + } +} +WARNING(property_name_chars_strict, check_property_name_chars_strict, PROPNODECHARSSTRICT); + #define DESCLABEL_FMT "%s%s%s%s%s" #define DESCLABEL_ARGS(node,prop,mark) \ ((mark) ? "value of " : ""), \ @@ -703,6 +736,8 @@ static struct check *check_table[] = { &address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell, &device_type_is_string, &model_is_string, &status_is_string, + &property_name_chars_strict, + &addr_size_cells, ®_format, &ranges_format, &unit_address_vs_reg,
While '?', '.', '+', '*', and '_' are considered valid characters their use is discouraged in recommended practices. '#' is also only recommended to be used at the beginning of property names. Testing this found one typo error with '.' used instead of ','. The rest of the warnings were all from underscores. Signed-off-by: Rob Herring <robh@kernel.org> --- checks.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html