Cleanup and remove verbosity #17
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/cc @ricarkol |
samoht
reviewed
Jun 2, 2017
lib/lifecycle.ml
Outdated
| @@ -1,3 +1,5 @@ | |||
| (* No shutdown events on Solo5. *) | |||
| let await_shutdown_request ?(can_poweroff:_) ?(can_reboot:_) () = | |||
| let _poweroff = can_poweroff in | |||
| let _rebooot = can_reboot in | |||
There was a problem hiding this comment.
It would be better to write:
let await_shutdown_request ?can_poweroff:_ ?can_reboot:_ () =e.g. the current form ignore the type, but what you want is ignoring the values (yes, that syntax is confusing).
|
The OCaml side LGTM appart one comment (I haven't looked at the C bindings very closely). |
Fix deprecation warning.
solo5_console_write() handles null bytes so no need to do strlen() here, just ask OCaml for the string length in bytes and pass that directly.
Formatting and more explicit if() chains. No functional change intended.
Not removing 4.02 for now since that would require an OPAM constraint and the package still builds with that version.
|
@samoht Rebased and addressed your comments. Thanks! |
hannesm
reviewed
Jun 4, 2017
| const char *str = String_val(arg); | ||
| solo5_console_write(str, strlen(str)); | ||
|
|
||
| solo5_console_write(String_val(arg), caml_string_length(arg)); |
There was a problem hiding this comment.
while your PR didn't change anything, I want to raise the issue that stub_console_write ignores the returned number of bytes written here -- see mirage/mirage-flow#4 (comment) for some more detail. I suspect we want stub_console_write to return an int, and loop until all the bytes are written.
|
LGTM! thanks! |
kit-ty-kate
pushed a commit
to kit-ty-kate/mirage-solo5
that referenced
this pull request
Oct 6, 2022
opam: Sync metadata with ocaml/opam-repository
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
General cleanup of the
mirage-solo5code. Removes the unnecessarySolo5: new bindingsmessage, cleans up formatting of other aborts/errors.I've also cleaned up some of the stubs (removing dead/unused/debug code) and fixed all the OCaml warnings with the exception of
Could someone with more OCaml/Lwt chops than me give this a quick review and verify that I've not broken anything? @hannesm / @samoht perhaps?