On Fri Sep 15 17:04:25 2023 +0000, Dmitry Timoshkov wrote:
I was thinking something like that:
if (flags & FLAG_1) size = max(size, FIELDOFFSET(..) + ...); if (flags & FLAG_2) size = max(size, ...); if (cbSize < size) { SetLastError(...); return; }To make it easier to read you can use just FIELDOFFSET() of the next field.
Thanks for the suggestion. It seems that it won't significantly improve readability or simplify the code, so I'd prefer to stick with my solution.
I think it's fine to duplicate that for A and W functions, and avoid changing Rtl* one at all, as long as we don't provide compatible interface for it, nothing is going to call it successfully anyway.
RtlCreateActivationContext() already performs the checks for the flags and structure field contents, so discussing where the checks belong is out of the scope of this MR. If you have justified reasons to doubt about current implementation feel free to add appropriate tests.
I was thinking about doing it like this:
``` 443 size = FIELD_OFFSET(ACTCTXA, wProcessorArchitecture); 444 if (actctx->dwFlags & ACTCTX_FLAG_PROCESSOR_ARCHITECTURE_VALID) 445 size = max(size, FIELD_OFFSET(ACTCTXA, wLangId)); 446 if (actctx->dwFlags & ACTCTX_FLAG_LANGID_VALID) 447 size = max(size, FIELD_OFFSET(ACTCTXA, lpAssemblyDirectory)); 448 if (actctx->dwFlags & ACTCTX_FLAG_ASSEMBLY_DIRECTORY_VALID) 449 size = max(size, FIELD_OFFSET(ACTCTXA, lpResourceName)); 450 if (actctx->dwFlags & ACTCTX_FLAG_RESOURCE_NAME_VALID) 451 size = max(size, FIELD_OFFSET(ACTCTXA, lpApplicationName)); 452 if (actctx->dwFlags & ACTCTX_FLAG_APPLICATION_NAME_VALID) 453 size = max(size, FIELD_OFFSET(ACTCTXA, hModule)); 454 if (actctx->dwFlags & ACTCTX_FLAG_HMODULE_VALID) 455 size = sizeof(ACTCTXA); 456 if (actctx->cbSize < size) 457 { 458 SetLastError(ERROR_INVALID_PARAMETER); 459 return INVALID_HANDLE_VALUE; 460 } ```
Then duplicating exactly the same check in CreateActCtxW(), structure layout is the same. This way we won't need six size checks and six jumps, times two.
Thanks for the bug report (https://bugs.winehq.org/show_bug.cgi?id=55782).