What's the expected result of wil::make_unique_string<unique_hlocal_string>(L"hello", 100); ?
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 3k
- Forks
- 300
- Avg merge
- 19h 12m
- Merged PRs (30d)
- 1
Description
auto str = wil::make_unique_string<unique_hlocal_string>(L"hello", 100);
auto equals = wcscmp(str.get(), L"hello");
Q: How big is the memory allocation supposed to be?
- 10 bytes
- 12 bytes
- 200 bytes
- 202 bytes
Unless I'm misreading something the answer is #4
length = 100. AllocatedBytes = (length + 1) * sizeof(wchar_t) == 202 bytes to hold 101 wide characters
str[0] = L'h'
str[1] = L'e'
str[2] = L'l'
str[3] = L'l'
str[4] = L'o'
str[5] = L'\0'
str[100] = L'\0'
str[6...99] are unassigned
Is this intentional?
As implemented specifying a length longer than the supplied string will allocated a larger buffer than necessary but the doccomment for resource.h line 3220+ and comments in the function don't mention this.
The same with a null parameter produces a similar result
auto str = wil::make_unique_string<unique_hlocal_string>(nullptr, 100);
length = 0, AllocatedBytes=202
str[0] = L'\0'
str[100] = L'\0'
str[1...99] are unassigned
No such mention in the doccomment but this is explicitly stated in the comment @ line 3246
// When the source string exists, calculate the number of characters to copy up to either
// 1) the length that is given
// 2) the length of the source string. When the source does not exist, use the given length
// for calculating both the size of allocated buffer and the number of characters to copy.
The results match #2
- "When the source does not exist" - source=nullptr so check
- "use the given length for calculating both the size of allocated buffer" - allocation=bytes for length+1 characters so check
- "and the number of characters to copy" - huh? "When the source does not exist, use the given length for calculating...the number of characters to copy". If we've got no source use length to compute how much source to copy?
Shouldn't this read
// 2) the length of the source string. When the source does not exist, use the given length
// for calculating the size of allocated buffer.
If length > wcslen(source) is supposed to allocate a larger buffer than strictly necessary for source the doccomment needs update. Also, that embedded comment "...number of characters to copy up to either" should be reworded to more clearly state the LARGER of the two will be used.
If the intent is to use min(length, wcslen(source)) then the code and that embedded comment needs update.
I found it a bit surprising behaviorally, given no mention in doccomments or hint in the interface e.g. there's no wil::make_unique_string(length) overload to just allocate a buffer of specified size.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read resource.h around the doccomment at line 3220 and the implementation comment near line 3246, then trace wil::make_unique_string with a longer source length and with nullptr. Resolve whether the current allocation behavior is intentional; done means the code and documentation consistently describe the chosen behavior, with any needed coverage added where the project’s existing tests for this utility live.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- tooling
- Issue type
- Documentation
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100