This diff adds memset_s(3) doc, which was discovered missing while working on another revision. It's been in two of my trees for a while and should be committed before inadvertently committed with something else.
Details
Details
- Reviewers
imp jhb glebius kib bjk cem - Group Reviewers
manpages - Commits
- rS329919: MFC r329361:
rS329361: Document memset_s(3). memset_s(3) is defined in
This diff has been in a couple of my trees, including my "prod" tree, for a while.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 14619
Event Timeline
| lib/libc/string/memset.3 | ||
|---|---|---|
| 71–74 | This case is a little weird and probably should be fully documented. If the overflow would occur, the memset() is still applied to the region of length destsz. Afterwards, the constraint handler / error is thrown. set_constraint_handler_s is probably worth documenting and Xring here. | |
Comment Actions
memset_s() is part of the C11 Annex K, which should be referenced there if man page describes the function.
| lib/libc/string/memset.3 | ||
|---|---|---|
| 70 | .Dv RSIZE_MAX | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 71–74 | Agreed that buffer overrun should probably be documented in memset.3. Trying to keep the description as brief as possible, does this look good? Access beyond the end of the b array will result in corruption of neigbouring memory (stack or heap). Unfortunately set_constraint_handler_s is not documented in our tree -- and yes it should be documented. Should this revision and the revision implementing gets_s (D12785) block pending documentation of set_constraint_handler_s? | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 71–74 |
No, that's inaccurate. Something like: If .Fn memset_s is called with len greater than destsz, the memset is first applied as if the len parameter was destsz. Then the error condition is thrown and the constraint handler invoked.
Right.
No, I don't think there's any particular reason to force an ordering. I'd just like it to be documented eventually. | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 60–63 | memset_s() also differs from memset() in that it is not removed as a dead store, making it useful for clearing sensitive data such as a password, like explicit_bzero(). | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 71–74 | FYI http://en.cppreference.com/w/c/string/byte/memset describes it well, I think:
| |
Comment Actions
All requested changes have been incorporated.
Unfortunately I might not be able to separate out memset_s changes from general memset doc changes prior to commit.
Let me know what you think.
| lib/libc/string/memset.3 | ||
|---|---|---|
| 99 | I can soften it a bit. How does this sound? For this reason it is not advised to use | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 99 | I think it is better to give advise what to do instead of what to avoid. For this reason it is advised to use memset_s(), instead of memset(), to clear subsequently unreferenced memory. For instance, a buffer containing the password should be cleared with memset_s() before free(). | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 99 | Agreed. Thank you. | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 71–74 | Is "thrown" really a conventional word to use in this context? To me it has more connotations of (e.g., C++) exceptions than a C library's internals. | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 71–74 | *Shrug*. constraint_handlers are really weird C APIs that no one uses, AFAIK. C11:
| |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 71–74 | "except that an error is returned, and the constraint handler is called, if" | |
Comment Actions
Can you please review this updated diff? (Hopefully I haven't lost track of all the changes.)
| lib/libc/string/memset.3 | ||
|---|---|---|
| 61 | or even "resulting from storage overflow"? | |
| 71 | maybe s/performs/behaves/? | |
| 76 | What is the .Sp macro? I don't think you need a macro to put a space after the comma; that should be the default behavior (multiple instances). | |
| 89 | The order of arguments to .Xr is the other way -- .Xr explicit_bzero 3 | |
| 91 | I might put a comma after "(DSE)". | |
| 96 | "On the other hand" might be too informal; an alternative would be "In contrast, .Fn memset may be optimized away if [...]" | |
| 101 | "subsequently unreferenced memory" is an unusual phrasing; I might say "to clear memory that will not subsequently be accessed". | |
| 102 | *a* password. | |
| 109 | This ("Undefined behavior...value of destsz") is not even a complete sentence. I agree with kib; just remove it and the next sentence (which is at best confusing). | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 61 | yes, from is better. overlay == overflow ... my exposure to IBM is beginning to show. | |
Comment Actions
I think we might be losing something in describing both functions in the same man page. Formatting it, it should read like this:
Comment Actions
As the man page is discussing two functions this may be confusing simply reading the markup. Here it is formatted:
DESCRIPTION
The memset() function writes len bytes of value c (converted to an unsigned char) to the string dest. Undefined behaviour from memset(), resulting from storage overflow, will occur if len is greater than the the length of buffer dest. The behaviour is also undefined if the pointer dest is NULL. The memset_s() function behaves the same as memset() except that an error is thrown and constraint handler is called if dest is a null pointer, destsz or len is greater than RSIZE_MAX, or len is greater than destsz (buffer overflow would occur). Like explicit_bzero(3), memset_s() is not removed through Dead Store Elimination (DSE), making it useful for clear‐ ing sensitve data. In contrast memset() function may be optimized away if the object modified by the function is not accessed again. To clear memory that will not subsequently be accessed it is advised to use memset_s() instead of memset(). For instance, a buffer containing a password should be cleared with memset_s() before free(3). dest < len <= destsz can cause a buffer overrun.
RETURN VALUES
The memset() function returns its first argument. The memset_s() func‐ tion returns zero on success, non-zero on error.
| lib/libc/string/memset.3 | ||
|---|---|---|
| 67 | I still do not like that the text mention NULL pointer as undefined behavior, but does not say that invalid pointer is an unhandled undefined behavior as well, even for memset_s(). | |
| 113 | I do not understand this sentence at all. Why do you compare pointers with length, what should it mean etc. | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 67 | I will reword this for memset(). memset_s() handles a NULL dest (memset_s.c line 48) but it does not handle dest being an invalid pointer. Also documented at http://en.cppreference.com/w/c/string/byte/memset. I agree that it should be documented that memset_s() will otherwise not handle an invalid pointer. | |
| 113 | This would be better said: A short However I say this in the previous paragraph. I'll simply remove it. | |
| lib/libc/string/memset.3 | ||
|---|---|---|
| 76 | Error handler is called first, and if it returns, the error is returned to the caller. The intent is that the handler does not return but performs the urgent process termination. | |
Comment Actions
Added sentence to say that the constraint handler doesn't necessarily return and if it does a return code is set.