2017-07-06 13:15 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
HRESULT d3dx_create_param_eval(struct d3dx9_base_effect *base_effect, void *byte_code, unsigned int byte_code_size, D3DXPARAMETER_TYPE type, - struct d3dx_param_eval **peval, ULONG64 *version_counter) DECLSPEC_HIDDEN; + struct d3dx_param_eval **peval, ULONG64 *version_counter, + const char **skip_constants_names, unsigned int skip_constant_name_count) DECLSPEC_HIDDEN;
I think you can get rid of the "_name" part in those parameters, for brevity.
+static HRESULT parse_skip_constants_string(char *skip_constants, const char ***names, + unsigned int *name_count)
It's a bit a matter of personal taste but triple pointers look quite ugly to me. In this case you could e.g. return "names" in place of HRESULT.
+{ + const char *name; + char *s; + unsigned int size = INITIAL_CONST_NAMES_SIZE; + + *names = HeapAlloc(GetProcessHeap(), 0, sizeof(*names) * size); + if (!*names) + return E_OUTOFMEMORY; + + *name_count = 0; + s = skip_constants; + while ((name = next_valid_constant_name(&s))) + { + if (*name_count == size) + { + size *= 2; + *names = HeapReAlloc(GetProcessHeap(), 0, *names, sizeof(*names) * size); + if (!*names) + return E_OUTOFMEMORY;
I think you're leaking memory on failure here.
+ } + (*names)[(*name_count)++] = name; + } + *names = HeapReAlloc(GetProcessHeap(), 0, *names, *name_count * sizeof(*names)); + if (!*names) + return E_OUTOFMEMORY;
And here.
+ for (i = 0; i < skip_constants_name_count; ++i) + { + struct d3dx_parameter *param; + + param = get_parameter_by_name(base, NULL, skip_constants_names[i]); + if (param) + { + for (j = 0; j < base->technique_count; ++j) + { + if (is_parameter_used(param, &base->techniques[j])) + { + WARN("Parameter %s found in skip_constants is used in technique %u.\n", + debugstr_a(skip_constants_names[i]), j); + HeapFree(GetProcessHeap(), 0, skip_constants_buffer); + HeapFree(GetProcessHeap(), 0, skip_constants_names); + d3dx9_base_effect_cleanup(base); + return D3DERR_INVALIDCALL; + } + } + } + else + { + TRACE("Parameter %s found in skip_constants not found.\n", + debugstr_a(skip_constants_names[i]));
This one sounds a bit awkward ("found... not found"). Maybe just rewrite that as "skip_constants parameter %s not found". You could apply the same trick to the previous message too.
+ for (j = 0; j < skip_constant_name_count; ++j) + { + if (!strcmp(cdesc[index].Name, skip_constants_names[j]))
I assume the skip_constants thing isn't used often and, when it is, very few constants (1-3) are in the list. If that's not the case, we might want to use an rbtree for the constant names.