really_write: remove deprecation and make robust against EINTR #29
Conversation
See discussion in https://ocaml.github.io/ocamlunix/ocamlunix.html#sec118 and xapi-project/xen-api#3570 Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
|
Really detailed comment above code, nice !!! 👍 |
|
Travis ain't happy :( |
lib/xapi-stdext-unix/unixext.ml
Outdated
|
|
||
| (* Ideally, really_write would be implemented with optional arguments ?(off=0) ?(len=String.length string) *) | ||
| let really_write_string fd string = | ||
| let payload = Bytes.unsafe_of_string string in |
There was a problem hiding this comment.
While you're in the area, could you remove this and use Unix.single_write_substring instead? I don't think we should use Bytes.unsafe_of_string anywhere unless we really have other choice. If you need to use an unsafe function to avoid code duplication it would be better to use Bytes.unsafe_to_string and implement really_write by using really_write_string, writing does not modify the buffer at all.
There was a problem hiding this comment.
Sure, I think it makes sense and will simplify the port to safe-string. Note however this will break the symmetry of the interface with respect to really_read (that requires a bytes buffer)
There was a problem hiding this comment.
That should be fine, read and write have to be different, the former does modify its argument.
If you want to make it clearer you can call the function really_write_substring, but its fine as it is.
|
The build failure is unrelated but weird. It is supposed to install lwt as it is a dependency of many of the packages, but for some reasons it does not pick it up as a dependency at all.... |
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
|
I've looked more carefully into the Travis error. It happens in one of the revdeps steps, for some reasons when installing |
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
|
The coverage build is now passing. The |
6 participants
See discussion in https://ocaml.github.io/ocamlunix/ocamlunix.html#sec118
and xapi-project/xen-api#3570
This also fixed another potential issue in
unixext_open_stubs