-
Notifications
You must be signed in to change notification settings - Fork 19
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
gluten-mirage: fix read and writev functions #32
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
profile = sparse | ||
profile = default | ||
break-cases = nested | ||
break-fun-decl = smart | ||
cases-exp-indent = 2 | ||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -32,54 +32,85 @@ | |||||||
|
||||||||
open Lwt.Infix | ||||||||
|
||||||||
module Make_IO (Flow : Mirage_flow.S) : | ||||||||
Gluten_lwt.IO with type socket = Flow.flow and type addr = unit = struct | ||||||||
type socket = Flow.flow | ||||||||
|
||||||||
type addr = unit | ||||||||
type 'a socket = { | ||||||||
flow : 'a; | ||||||||
mutable buf : Cstruct.t; | ||||||||
} | ||||||||
|
||||||||
let shutdown flow = Flow.close flow | ||||||||
let create_socket flow = { flow; buf = Cstruct.empty } | ||||||||
|
||||||||
let shutdown_receive flow = Lwt.async (fun () -> shutdown flow) | ||||||||
module Make_IO (Flow : Mirage_flow.S) : | ||||||||
Gluten_lwt.IO with type socket = Flow.flow socket and type addr = unit = | ||||||||
struct | ||||||||
type nonrec socket = Flow.flow socket | ||||||||
type addr = unit | ||||||||
|
||||||||
let shutdown sock = Flow.close sock.flow | ||||||||
let shutdown_receive sock = Lwt.async (fun () -> shutdown sock) | ||||||||
let close = shutdown | ||||||||
|
||||||||
let read flow bigstring ~off ~len:_ = | ||||||||
let buffered_read sock len = | ||||||||
let trunc buf = | ||||||||
match Cstruct.length buf > len with | ||||||||
| false -> | ||||||||
buf | ||||||||
| true -> | ||||||||
let head, rest = Cstruct.split buf len in | ||||||||
sock.buf <- rest; | ||||||||
head | ||||||||
in | ||||||||
let buffered_data = | ||||||||
match Cstruct.is_empty sock.buf with | ||||||||
| true -> | ||||||||
None | ||||||||
| false -> | ||||||||
let buf = sock.buf in | ||||||||
sock.buf <- Cstruct.empty; | ||||||||
Some (Ok (`Data (trunc buf))) | ||||||||
in | ||||||||
match buffered_data with | ||||||||
| Some data -> | ||||||||
Lwt.return data | ||||||||
| None -> ( | ||||||||
Flow.read sock.flow >|= fun data -> | ||||||||
assert (Cstruct.is_empty sock.buf); | ||||||||
match data with Ok (`Data buf) -> Ok (`Data (trunc buf)) | x -> x) | ||||||||
|
||||||||
let read sock bigstring ~off ~len = | ||||||||
Lwt.catch | ||||||||
(fun () -> | ||||||||
Flow.read flow >|= function | ||||||||
buffered_read sock len >|= function | ||||||||
| Ok (`Data buf) -> | ||||||||
Bigstringaf.blit | ||||||||
buf.buffer | ||||||||
~src_off:buf.off | ||||||||
bigstring | ||||||||
~dst_off:off | ||||||||
Bigstringaf.blit buf.buffer ~src_off:buf.off bigstring ~dst_off:off | ||||||||
~len:buf.len; | ||||||||
`Ok buf.len | ||||||||
| Ok `Eof -> | ||||||||
`Eof | ||||||||
| Error error -> | ||||||||
failwith (Format.asprintf "%a" Flow.pp_error error)) | ||||||||
(fun exn -> shutdown flow >>= fun () -> Lwt.fail exn) | ||||||||
(fun exn -> shutdown sock >>= fun () -> Lwt.fail exn) | ||||||||
|
||||||||
let writev flow iovecs = | ||||||||
let writev sock iovecs = | ||||||||
let cstruct_iovecs = | ||||||||
List.map | ||||||||
(fun { Faraday.buffer; off; len } -> | ||||||||
Cstruct.of_bigarray ~off ~len buffer) | ||||||||
iovecs | ||||||||
in | ||||||||
let len = Cstruct.lenv cstruct_iovecs in | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be easier to create the copy inside the |
||||||||
let data = Cstruct.create_unsafe len in | ||||||||
let _, _ = Cstruct.fillv ~src:cstruct_iovecs ~dst:data in | ||||||||
Comment on lines
+103
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
Lwt.catch | ||||||||
(fun () -> | ||||||||
Flow.writev flow cstruct_iovecs >|= fun x -> | ||||||||
Flow.write sock.flow data >|= fun x -> | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
match x with | ||||||||
| Ok () -> | ||||||||
`Ok (Cstruct.lenv cstruct_iovecs) | ||||||||
| Error `Closed -> | ||||||||
`Closed | ||||||||
| Error other_error -> | ||||||||
raise (Failure (Format.asprintf "%a" Flow.pp_write_error other_error))) | ||||||||
(fun exn -> shutdown flow >>= fun () -> Lwt.fail exn) | ||||||||
(fun exn -> shutdown sock >>= fun () -> Lwt.fail exn) | ||||||||
end | ||||||||
|
||||||||
module Server (Flow : Mirage_flow.S) = Gluten_lwt.Server (Make_IO (Flow)) | ||||||||
|
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.
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.
But that (potentially) allocates many small
Cstruct
s, my version allocates only one biggerCstruct
and copies everything into it.