Skip to content

Commit d5637bb

Browse files
committed
Add some type checking of configs
Before, the types were not checked and just expected. The old behavior would cause lots of tracebacks, or, much worse, convert things like: ``` { "target_overrides": { "*": { "target.macros_add": "CONFIG_GPIO_AS_PINRESET" } } } ``` into a definition of each of the letters as macros that expand to nothing, causing massive compilation problems. I resolved this by adding some type checking to the config data. Now there is a type check for most of the fields within a library and application configurations.
1 parent 75f6f2d commit d5637bb

File tree

1 file changed

+41
-16
lines changed

1 file changed

+41
-16
lines changed

tools/config.py

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,12 @@ def _process_macros(mlist, macros, unit_name, unit_kind):
332332
macros[macro.macro_name] = macro
333333

334334

335+
def check_dict_types(dict, type_dict, dict_loc):
336+
for key, value in dict.iteritems():
337+
if not isinstance(value, type_dict[key]):
338+
raise ConfigException("The value of %s.%s is not of type %s" %
339+
(dict_loc, key, type_dict[key].__name__))
340+
335341
Region = namedtuple("Region", "name start size active filename")
336342

337343
class Config(object):
@@ -342,14 +348,14 @@ class Config(object):
342348
__mbed_app_config_name = "mbed_app.json"
343349
__mbed_lib_config_name = "mbed_lib.json"
344350

345-
# Allowed keys in configuration dictionaries
351+
# Allowed keys in configuration dictionaries, and their types
346352
# (targets can have any kind of keys, so this validation is not applicable
347353
# to them)
348354
__allowed_keys = {
349-
"library": set(["name", "config", "target_overrides", "macros",
350-
"__config_path"]),
351-
"application": set(["config", "target_overrides",
352-
"macros", "__config_path"])
355+
"library": {"name": str, "config": dict, "target_overrides": dict,
356+
"macros": list, "__config_path": str},
357+
"application": {"config": dict, "target_overrides": dict,
358+
"macros": list, "__config_path": str}
353359
}
354360

355361
__unused_overrides = set(["target.bootloader_img", "target.restrict_size"])
@@ -398,11 +404,13 @@ def __init__(self, tgt, top_level_dirs=None, app_config=None):
398404

399405
# Check the keys in the application configuration data
400406
unknown_keys = set(self.app_config_data.keys()) - \
401-
self.__allowed_keys["application"]
407+
set(self.__allowed_keys["application"].keys())
402408
if unknown_keys:
403409
raise ConfigException("Unknown key(s) '%s' in %s" %
404410
(",".join(unknown_keys),
405411
self.__mbed_app_config_name))
412+
check_dict_types(self.app_config_data, self.__allowed_keys["application"],
413+
"app-config")
406414
# Update the list of targets with the ones defined in the application
407415
# config, if applicable
408416
self.lib_config_data = {}
@@ -542,19 +550,34 @@ def _process_config_and_overrides(self, data, params, unit_name, unit_kind):
542550
# Parse out cumulative overrides
543551
for attr, cumulatives in self.cumulative_overrides.iteritems():
544552
if 'target.'+attr in overrides:
545-
cumulatives.strict_cumulative_overrides(
546-
overrides['target.'+attr])
547-
del overrides['target.'+attr]
553+
key = 'target.' + attr
554+
if not isinstance(overrides[key], list):
555+
raise ConfigException(
556+
"The value of %s.%s is not of type %s" %
557+
(unit_name, "target_overrides." + key,
558+
"list"))
559+
cumulatives.strict_cumulative_overrides(overrides[key])
560+
del overrides[key]
548561

549562
if 'target.'+attr+'_add' in overrides:
550-
cumulatives.add_cumulative_overrides(
551-
overrides['target.'+attr+'_add'])
552-
del overrides['target.'+attr+'_add']
563+
key = 'target.' + attr + "_add"
564+
if not isinstance(overrides[key], list):
565+
raise ConfigException(
566+
"The value of %s.%s is not of type %s" %
567+
(unit_name, "target_overrides." + key,
568+
"list"))
569+
cumulatives.add_cumulative_overrides(overrides[key])
570+
del overrides[key]
553571

554572
if 'target.'+attr+'_remove' in overrides:
555-
cumulatives.remove_cumulative_overrides(
556-
overrides['target.'+attr+'_remove'])
557-
del overrides['target.'+attr+'_remove']
573+
key = 'target.' + attr + "_remove"
574+
if not isinstance(overrides[key], list):
575+
raise ConfigException(
576+
"The value of %s.%s is not of type %s" %
577+
(unit_name, "target_overrides." + key,
578+
"list"))
579+
cumulatives.remove_cumulative_overrides(overrides[key])
580+
del overrides[key]
558581

559582
# Consider the others as overrides
560583
for name, val in overrides.items():
@@ -639,10 +662,12 @@ def get_lib_config_data(self):
639662
"""
640663
all_params, macros = {}, {}
641664
for lib_name, lib_data in self.lib_config_data.items():
642-
unknown_keys = set(lib_data.keys()) - self.__allowed_keys["library"]
665+
unknown_keys = (set(lib_data.keys()) -
666+
set(self.__allowed_keys["library"].keys()))
643667
if unknown_keys:
644668
raise ConfigException("Unknown key(s) '%s' in %s" %
645669
(",".join(unknown_keys), lib_name))
670+
check_dict_types(lib_data, self.__allowed_keys["library"], lib_name)
646671
all_params.update(self._process_config_and_overrides(lib_data, {},
647672
lib_name,
648673
"library"))

0 commit comments

Comments
 (0)