Skip to content

Conversation

@mwinterb
Copy link
Contributor

@mwinterb mwinterb commented Mar 3, 2016

This is for non-'Desktop' applications that have a more limited collection of functions.
Addresses #280.

This is for non-'Desktop' applications that have a
more limited collection of functions.
@vitaut
Copy link
Contributor

vitaut commented Mar 3, 2016

Thanks for the PR, but why not free the buffer with HeapFree (GetProcessHeap(), allocatedMessage) as suggested in https://msdn.microsoft.com/en-us/library/windows/desktop/ms679351(v=vs.85).aspx instead of increasing the buffer size in a loop?

@mwinterb
Copy link
Contributor Author

mwinterb commented Mar 3, 2016

The documentation for the define indirectly calls out that it wasn't available for "Windows Store apps" for Windows 8 and 8.1 and the latest Windows SDK only defines it for "desktop family" applications. #280 also pointed out that FORMAT_MESSAGE_ALLOCATE_BUFFER was not defined. It not being defined for non-desktop family applications if Windows 10 is being targeted is probably a Windows header file bug, but cppformat would still have to deal with old SDKs even if that gets fixed.

So, basically, just to maximize compatibility with as many different Windows "versions" as possible.

I need to also amend this PR with a test to make sure that TBS_E_PROVISIONING_NOT_ALLOWED works consistently with directly calling FormatMessageW(ALLOCATE) since that error code forces the retry, I just wasn't aware of that case when I started.

@vitaut
Copy link
Contributor

vitaut commented Mar 3, 2016

Fair enough. I'm not quite sure what TBS_E_PROVISIONING_NOT_ALLOWED is about but I'll wait for you to update the PR.

@mwinterb
Copy link
Contributor Author

mwinterb commented Mar 3, 2016

The message text for TBS_E_PROVISIONING_NOT_ALLOWED is more than 500 characters long, so it'll just be to verify that the buffer is too small case is handled correctly.

vitaut added a commit that referenced this pull request Mar 6, 2016
Changed format_windows_error to not need LocalFree
@vitaut vitaut merged commit 6883d6e into fmtlib:master Mar 6, 2016
@vitaut
Copy link
Contributor

vitaut commented Mar 6, 2016

Merged, thanks!

@mwinterb mwinterb deleted the winerror_winu branch March 18, 2016 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants