layers: Improved generated struct validation code
Improvements to the parameter validation code generator, primarily to
eliminate string concatenations performed when processing structs. A
validation function was being produced for each struct, which received a
string specifying the name of the struct parameter. This string was then
concatenated with the names of the struct members. The struct validation
code is no longer in a separate function, and a single string containing
the full names of each struct member (eg. pCreateInfo->pApplicationInfo)
is created at code generation time.
Change-Id: I2800bfbc26247e4e4c6f8c885c0e7c4b8b3776a2
diff --git a/generator.py b/generator.py
index a96ada8..f02419f 100644
--- a/generator.py
+++ b/generator.py
@@ -2852,7 +2852,7 @@
self.structTypes = dict() # Map of Vulkan struct typename to required VkStructureType
self.commands = [] # List of CommandData records for all Vulkan commands
self.structMembers = [] # List of StructMemberData records for all Vulkan structs
- self.validatedStructs = set() # Set of structs containing members that require validation
+ self.validatedStructs = dict() # Map of structs type names to generated validation code for that struct type
self.enumRanges = dict() # Map of enum name to BEGIN/END range values
# Named tuples to store struct and command data
self.StructType = namedtuple('StructType', ['name', 'value'])
@@ -2922,7 +2922,7 @@
self.structTypes = dict()
self.commands = []
self.structMembers = []
- self.validatedStructs = set()
+ self.validatedStructs = dict()
self.enumRanges = dict()
def endFeature(self):
# C-specific
@@ -2935,7 +2935,6 @@
if (self.featureExtraProtect != None):
write('#ifdef', self.featureExtraProtect, file=self.outFile)
# Generate the struct member checking code from the captured data
- self.prepareStructMemberData()
self.processStructMemberData()
# Generate the command parameter checking code from the captured data
self.processCmdData()
@@ -3265,15 +3264,15 @@
# This is an array with a pointer to a count value
if lenValue.ispointer:
# When the length parameter is a pointer, there is an extra Boolean parameter in the function call to indicate if it is required
- checkExpr.append('skipCall |= validate_struct_type_array(report_data, {}, {ldn}, {dn}, "{sv}", {pf}{ln}, {pf}{vn}, {sv}, {}, {}, {});\n'.format(
+ checkExpr.append('skipCall |= validate_struct_type_array(report_data, "{}", "{ldn}", "{dn}", "{sv}", {pf}{ln}, {pf}{vn}, {sv}, {}, {}, {});\n'.format(
funcPrintName, lenPtrRequired, lenValueRequired, valueRequired, ln=lenValue.name, ldn=lenPrintName, dn=valuePrintName, vn=value.name, sv=stype.value, pf=prefix))
# This is an array with an integer count value
else:
- checkExpr.append('skipCall |= validate_struct_type_array(report_data, {}, {ldn}, {dn}, "{sv}", {pf}{ln}, {pf}{vn}, {sv}, {}, {});\n'.format(
+ checkExpr.append('skipCall |= validate_struct_type_array(report_data, "{}", "{ldn}", "{dn}", "{sv}", {pf}{ln}, {pf}{vn}, {sv}, {}, {});\n'.format(
funcPrintName, lenValueRequired, valueRequired, ln=lenValue.name, ldn=lenPrintName, dn=valuePrintName, vn=value.name, sv=stype.value, pf=prefix))
# This is an individual struct
else:
- checkExpr.append('skipCall |= validate_struct_type(report_data, {}, {}, "{sv}", {}{vn}, {sv}, {});\n'.format(
+ checkExpr.append('skipCall |= validate_struct_type(report_data, "{}", "{}", "{sv}", {}{vn}, {sv}, {});\n'.format(
funcPrintName, valuePrintName, prefix, valueRequired, vn=value.name, sv=stype.value))
return checkExpr
#
@@ -3290,7 +3289,7 @@
extStructCount = 'ARRAY_SIZE(allowedStructs)'
extStructVar = 'allowedStructs'
extStructNames = '"' + ', '.join(structs) + '"'
- checkExpr.append('skipCall |= validate_struct_pnext(report_data, {}, {}, {}, {}{}, {}, {});\n'.format(
+ checkExpr.append('skipCall |= validate_struct_pnext(report_data, "{}", "{}", {}, {}{}, {}, {});\n'.format(
funcPrintName, valuePrintName, extStructNames, prefix, value.name, extStructCount, extStructVar))
return checkExpr
#
@@ -3303,16 +3302,16 @@
# If count and array parameters are optional, there will be no validation
if valueRequired == 'true' or lenPtrRequired == 'true' or lenValueRequired == 'true':
# When the length parameter is a pointer, there is an extra Boolean parameter in the function call to indicate if it is required
- checkExpr.append('skipCall |= validate_array(report_data, {}, {ldn}, {dn}, {pf}{ln}, {pf}{vn}, {}, {}, {});\n'.format(
+ checkExpr.append('skipCall |= validate_array(report_data, "{}", "{ldn}", "{dn}", {pf}{ln}, {pf}{vn}, {}, {}, {});\n'.format(
funcPrintName, lenPtrRequired, lenValueRequired, valueRequired, ln=lenValue.name, ldn=lenPrintName, dn=valuePrintName, vn=value.name, pf=prefix))
# This is an array with an integer count value
else:
# If count and array parameters are optional, there will be no validation
if valueRequired == 'true' or lenValueRequired == 'true':
# Arrays of strings receive special processing
- funcName = 'validate_array' if value.type != 'char' else 'validate_string_array'
- checkExpr.append('skipCall |= {}(report_data, {}, {ldn}, {dn}, {pf}{ln}, {pf}{vn}, {}, {});\n'.format(
- funcName, funcPrintName, lenValueRequired, valueRequired, ln=lenValue.name, ldn=lenPrintName, dn=valuePrintName, vn=value.name, pf=prefix))
+ validationFuncName = 'validate_array' if value.type != 'char' else 'validate_string_array'
+ checkExpr.append('skipCall |= {}(report_data, "{}", "{ldn}", "{dn}", {pf}{ln}, {pf}{vn}, {}, {});\n'.format(
+ validationFuncName, funcPrintName, lenValueRequired, valueRequired, ln=lenValue.name, ldn=lenPrintName, dn=valuePrintName, vn=value.name, pf=prefix))
if checkExpr:
if lenValue and ('->' in lenValue.name):
# Add checks to ensure the validation call does not dereference a NULL pointer to obtain the count
@@ -3321,22 +3320,69 @@
elif not value.isoptional:
# Function pointers need a reinterpret_cast to void*
if value.type[:4] == 'PFN_':
- checkExpr.append('skipCall |= validate_required_pointer(report_data, {}, {}, reinterpret_cast<const void*>({}{}));\n'.format(funcPrintName, valuePrintName, prefix, value.name))
+ checkExpr.append('skipCall |= validate_required_pointer(report_data, "{}", "{}", reinterpret_cast<const void*>({}{}));\n'.format(funcPrintName, valuePrintName, prefix, value.name))
else:
- checkExpr.append('skipCall |= validate_required_pointer(report_data, {}, {}, {}{});\n'.format(funcPrintName, valuePrintName, prefix, value.name))
+ checkExpr.append('skipCall |= validate_required_pointer(report_data, "{}", "{}", {}{});\n'.format(funcPrintName, valuePrintName, prefix, value.name))
return checkExpr
#
+ # Process struct member validation code, performing name suibstitution if required
+ def processStructMemberCode(self, line, funcName, memberNamePrefix, memberDisplayNamePrefix):
+ if any(token in line for token in ['{funcName}', '{valuePrefix}', '{displayNamePrefix}']):
+ return line.format(funcName=funcName, valuePrefix=memberNamePrefix, displayNamePrefix=memberDisplayNamePrefix)
+ return line
+ #
+ # Process struct validation code for inclusion in function or parent struct validation code
+ def expandStructCode(self, lines, funcName, memberNamePrefix, memberDisplayNamePrefix, indent, output):
+ for line in lines:
+ if output:
+ output[-1] += '\n'
+ if type(line) is list:
+ for sub in line:
+ output.append(self.processStructMemberCode(indent + sub, funcName, memberNamePrefix, memberDisplayNamePrefix))
+ else:
+ output.append(self.processStructMemberCode(indent + line, funcName, memberNamePrefix, memberDisplayNamePrefix))
+ return output
+ #
+ # Process struct pointer/array validation code, perfoeming name substitution if required
+ def expandStructPointerCode(self, prefix, value, lenValue, funcName, valueDisplayName):
+ expr = []
+ expr.append('if ({}{} != NULL)\n'.format(prefix, value.name))
+ expr.append('{')
+ indent = self.incIndent(None)
+ if lenValue:
+ # Need to process all elements in the array
+ indexName = lenValue.name.replace('Count', 'Index')
+ expr[-1] += '\n'
+ expr.append(indent + 'for (uint32_t {iname} = 0; {iname} < {}{}; ++{iname})\n'.format(prefix, lenValue.name, iname=indexName))
+ expr.append(indent + '{')
+ indent = self.incIndent(indent)
+ # Prefix for value name to display in error message
+ memberNamePrefix = '{}{}[{}].'.format(prefix, value.name, indexName)
+ memberDisplayNamePrefix = '{}[i].'.format(valueDisplayName)
+ else:
+ memberNamePrefix = '{}{}->'.format(prefix, value.name)
+ memberDisplayNamePrefix = '{}->'.format(valueDisplayName)
+ #
+ # Expand the struct validation lines
+ expr = self.expandStructCode(self.validatedStructs[value.type], funcName, memberNamePrefix, memberDisplayNamePrefix, indent, expr)
+ #
+ if lenValue:
+ # Close if and for scopes
+ indent = self.decIndent(indent)
+ expr.append(indent + '}\n')
+ expr.append('}\n')
+ return expr
+ #
# Generate the parameter checking code
def genFuncBody(self, funcName, values, valuePrefix, displayNamePrefix, structTypeName):
lines = [] # Generated lines of code
unused = [] # Unused variable names
for value in values:
- usedAlways = []
- usedOnInput = []
+ usedLines = []
lenParam = None
#
# Generate the full name of the value, which will be printed in the error message, by adding the variable prefix to the value name
- valueDisplayName = '(std::string({}) + std::string("{}")).c_str()'.format(displayNamePrefix, value.name) if displayNamePrefix else '"{}"'.format(value.name)
+ valueDisplayName = '{}{}'.format(displayNamePrefix, value.name)
#
# Check for NULL pointers, ignore the inout count parameters that
# will be validated with their associated array
@@ -3354,7 +3400,7 @@
if value.len:
# The parameter is an array with an explicit count parameter
lenParam = self.getLenParam(values, value.len)
- lenDisplayName = '(std::string({}) + std::string("{}")).c_str()'.format(displayNamePrefix, lenParam.name) if displayNamePrefix else '"{}"'.format(lenParam.name)
+ lenDisplayName = '{}{}'.format(displayNamePrefix, lenParam.name)
if lenParam.ispointer:
# Count parameters that are pointers are inout
if type(lenParam.isoptional) is list:
@@ -3371,138 +3417,57 @@
#
# If this is a pointer to a struct with an sType field, verify the type
if value.type in self.structTypes:
- usedAlways += self.makeStructTypeCheck(valuePrefix, value, lenParam, req, cvReq, cpReq, funcName, lenDisplayName, valueDisplayName)
+ usedLines += self.makeStructTypeCheck(valuePrefix, value, lenParam, req, cvReq, cpReq, funcName, lenDisplayName, valueDisplayName)
elif value.name == 'pNext':
# We need to ignore VkDeviceCreateInfo and VkInstanceCreateInfo, as the loader manipulates them in a way that is not documented in vk.xml
if not structTypeName in ['VkDeviceCreateInfo', 'VkInstanceCreateInfo']:
- usedAlways += self.makeStructNextCheck(valuePrefix, value, funcName, valueDisplayName)
+ usedLines += self.makeStructNextCheck(valuePrefix, value, funcName, valueDisplayName)
else:
- usedAlways += self.makePointerCheck(valuePrefix, value, lenParam, req, cvReq, cpReq, funcName, lenDisplayName, valueDisplayName)
+ usedLines += self.makePointerCheck(valuePrefix, value, lenParam, req, cvReq, cpReq, funcName, lenDisplayName, valueDisplayName)
#
# If this is a pointer to a struct (input), see if it contains members that need to be checked
if value.type in self.validatedStructs and value.isconst:
- #
- # The name prefix used when reporting an error with a struct member (eg. the 'pCreateInfor->' in 'pCreateInfo->sType')
- if lenParam:
- prefix = '(std::string({}) + std::string("{}[i]->")).c_str()'.format(displayNamePrefix, value.name) if displayNamePrefix else '(std::string("{}[i]->")).c_str()'.format(value.name)
- else:
- prefix = '(std::string({}) + std::string("{}->")).c_str()'.format(displayNamePrefix, value.name) if displayNamePrefix else '"{}->"'.format(value.name)
- # Validation function does not have an isInput field
- if lenParam:
- expr = []
- # Need to process all elements in the array
- expr.append('if ({}{} != NULL) {{\n'.format(valuePrefix, value.name))
- indent = self.incIndent(None)
- expr.append(indent + 'for (uint32_t i = 0; i < {}{}; ++i) {{\n'.format(valuePrefix, lenParam.name))
- indent = self.incIndent(indent)
- expr.append(indent + 'skipCall |= parameter_validation_{}(report_data, {}, {}, &({}{}[i]));\n'.format(value.type, funcName, prefix, valuePrefix, value.name))
- indent = self.decIndent(indent)
- expr.append(indent + '}\n')
- indent = self.decIndent(indent)
- expr.append(indent + '}\n')
- else:
- expr = 'skipCall |= parameter_validation_{}(report_data, {}, {}, {}{});\n'.format(value.type, funcName, prefix, valuePrefix, value.name)
- usedAlways.append(expr)
+ usedLines.append(self.expandStructPointerCode(valuePrefix, value, lenParam, funcName, valueDisplayName))
elif value.isbool and value.isconst:
- usedOnInput.append('skipCall |= validate_bool32_array(report_data, {}, {}, {pf}{}, {pf}{});\n'.format(funcName, valueDisplayName, lenParam.name, value.name, pf=valuePrefix))
+ usedLines.append('skipCall |= validate_bool32_array(report_data, "{}", "{}", {pf}{}, {pf}{});\n'.format(funcName, valueDisplayName, lenParam.name, value.name, pf=valuePrefix))
elif value.israngedenum and value.isconst:
enumRange = self.enumRanges[value.type]
- usedOnInput.append('skipCall |= validate_ranged_enum_array(report_data, {}, {}, "{}", {}, {}, {pf}{}, {pf}{});\n'.format(funcName, valueDisplayName, value.type, enumRange[0], enumRange[1], lenParam.name, value.name, pf=valuePrefix))
+ usedLines.append('skipCall |= validate_ranged_enum_array(report_data, "{}", "{}", "{}", {}, {}, {pf}{}, {pf}{});\n'.format(funcName, valueDisplayName, value.type, enumRange[0], enumRange[1], lenParam.name, value.name, pf=valuePrefix))
+ # Non-pointer types
elif value.type in self.validatedStructs:
- # The name of the value with prefix applied
- prefix = '(std::string({}) + std::string("{}.")).c_str()'.format(displayNamePrefix, value.name) if displayNamePrefix else '"{}."'.format(value.name)
- usedAlways.append('skipCall |= parameter_validation_{}(report_data, {}, {}, &({}{}));\n'.format(value.type, funcName, prefix, valuePrefix, value.name))
+ memberNamePrefix = '{}{}.'.format(valuePrefix, value.name)
+ memberDisplayNamePrefix = '{}.'.format(valueDisplayName)
+ usedLines.append(self.expandStructCode(self.validatedStructs[value.type], funcName, memberNamePrefix, memberDisplayNamePrefix, '', []))
elif value.isbool:
- usedOnInput.append('skipCall |= validate_bool32(report_data, {}, {}, {}{});\n'.format(funcName, valueDisplayName, valuePrefix, value.name))
+ usedLines.append('skipCall |= validate_bool32(report_data, "{}", "{}", {}{});\n'.format(funcName, valueDisplayName, valuePrefix, value.name))
elif value.israngedenum:
enumRange = self.enumRanges[value.type]
- usedOnInput.append('skipCall |= validate_ranged_enum(report_data, {}, {}, "{}", {}, {}, {}{});\n'.format(funcName, valueDisplayName, value.type, enumRange[0], enumRange[1], valuePrefix, value.name))
+ usedLines.append('skipCall |= validate_ranged_enum(report_data, "{}", "{}", "{}", {}, {}, {}{});\n'.format(funcName, valueDisplayName, value.type, enumRange[0], enumRange[1], valuePrefix, value.name))
#
# Append the parameter check to the function body for the current command
- if usedAlways or usedOnInput:
- # Both used always and used on input are currently treated the same
- if usedAlways:
- lines += usedAlways
- if usedOnInput:
- lines += usedOnInput
+ if usedLines:
+ lines += usedLines
elif not value.iscount:
# If no expression was generated for this value, it is unreferenced by the validation function, unless
# it is an array count, which is indirectly referenced for array valiadation.
unused.append(value.name)
return lines, unused
#
- # Post-process the collected struct member data to create a list of structs
- # with members that need to be validated
- def prepareStructMemberData(self):
- for struct in self.structMembers:
- validated = False
- for member in struct.members:
- # Counts will be validated with their associated array
- if not member.iscount:
- lenParam = self.getLenParam(struct.members, member.len)
- # The sType value needs to be validated
- # The pNext value needs to be validated
- # A required array/count needs to be validated
- # A required pointer needs to be validated
- # A bool needs to be validated, and the struct is an input parameter
- # An enum needs to be validated, and the struct is an input parameter
- if member.type in self.structTypes:
- validated = True
- elif member.name == 'pNext':
- validated = True
- elif member.ispointer and lenParam: # This is an array
- # Make sure len is not optional
- if lenParam.ispointer:
- if not lenParam.isoptional[0] or not lenParam.isoptional[1] or not member.isoptional:
- validated = True
- else:
- if not lenParam.isoptional or not member.isoptional:
- validated = True
- elif member.ispointer and not member.isoptional:
- validated = True
- elif member.isbool or member.israngedenum:
- validated = True
- #
- if validated:
- self.validatedStructs.add(struct.name)
- #
# Generate the struct member check code from the captured data
def processStructMemberData(self):
indent = self.incIndent(None)
for struct in self.structMembers:
#
# The string returned by genFuncBody will be nested in an if check for a NULL pointer, so needs its indent incremented
- lines, unused = self.genFuncBody('pFuncName', struct.members, 'pStruct->', 'pVariableName', struct.name)
+ lines, unused = self.genFuncBody('{funcName}', struct.members, '{valuePrefix}', '{displayNamePrefix}', struct.name)
if lines:
- cmdDef = 'static bool parameter_validation_{}(\n'.format(struct.name)
- cmdDef += ' debug_report_data*'.ljust(self.genOpts.alignFuncParam) + ' report_data,\n'
- cmdDef += ' const char*'.ljust(self.genOpts.alignFuncParam) + ' pFuncName,\n'
- cmdDef += ' const char*'.ljust(self.genOpts.alignFuncParam) + ' pVariableName,\n'
- cmdDef += ' const {}*'.format(struct.name).ljust(self.genOpts.alignFuncParam) + ' pStruct)\n'
- cmdDef += '{\n'
- cmdDef += indent + 'bool skipCall = false;\n'
- cmdDef += '\n'
- cmdDef += indent + 'if (pStruct != NULL) {'
- indent = self.incIndent(indent)
- for line in lines:
- cmdDef += '\n'
- if type(line) is list:
- for sub in line:
- cmdDef += indent + sub
- else:
- cmdDef += indent + line
- indent = self.decIndent(indent)
- cmdDef += indent +'}\n'
- cmdDef += '\n'
- cmdDef += indent + 'return skipCall;\n'
- cmdDef += '}\n'
- self.appendSection('command', cmdDef)
+ self.validatedStructs[struct.name] = lines
#
# Generate the command param check code from the captured data
def processCmdData(self):
indent = self.incIndent(None)
for command in self.commands:
- lines, unused = self.genFuncBody('"{}"'.format(command.name), command.params, '', None, None)
+ lines, unused = self.genFuncBody(command.name, command.params, '', '', None)
if lines:
cmdDef = self.getCmdDef(command) + '\n'
cmdDef += '{\n'