-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
doc: document unspecified behavior for buf.write* methods #5925
Conversation
Per nodejs#1161, when the buf.write*() methods are given anything other than what they expect, indicate that the behavior is unspecified. Fixes: nodejs#1161
@@ -1397,7 +1397,8 @@ console.log(`${len} bytes: ${buf.toString('utf8', 0, len)}`); | |||
|
|||
Writes `value` to the Buffer at the specified `offset` with specified endian | |||
format (`writeDoubleBE()` writes big endian, `writeDoubleLE()` writes little | |||
endian). The `value` argument must be a valid 64-bit double. | |||
endian). The `value` argument *should* be a valid 64-bit double. Behavior is | |||
unspecified if `value` is anything other than a 64-bit double. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe undefined is better than unspecified here? we're specifying that the behavior is undefined right here. That's what most other languages do (like C++).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "Behavior is not defined" would be better, since the word "undefined" might confuse basic level english speakers into thinking undefined
will get written into the buffer somehow. Though TBH "unspecified" looks fine to me.
LGTM though we need some more people to weigh in on the choice of word (unspecified/undefined/not defined) |
4a3bc64
to
1aeb24f
Compare
@claudiorodriguez @benjamingr .. updated. PTAL |
LGTM |
Landed in 64bf4b3 |
Pull Request check-list
existing APIs, or introduces new ones)?
Affected core subsystem(s)
doc/api/buffer.markdown
Description of change
Per #1161, when the buf.write*() methods are given anything other than what they expect, indicate that the behavior is unspecified.
Fixes: #1161