Skip to content

Conversation

Julow
Copy link
Collaborator

@Julow Julow commented Feb 25, 2025

On top of #1315

Remove many misuses of input and output channels.

Copy link
Collaborator

@panglesd panglesd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Much better!

module Json = Json

module Io_utils = struct
let _with_resource res ~close f =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this starting with an underscore to indicate it's not for use outside of the module?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. This avoids writing a signature to abstract this one function while the others are trivial.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added some documentation to the module to make it easier to read without an interface.

panglesd and others added 5 commits February 26, 2025 10:50
Input or output channels used for marshalling were often left unclosed.
This can cause resource leaks when a piece of code that was once run
once is suddenly called multiple times.
This adds safe function for reading from and writing to files and
replaces code that did unsafe resource manipulations.
@Julow Julow force-pushed the safe_file_handling branch from ab3aa8e to 21826bb Compare February 26, 2025 09:51
@Julow Julow added the no changelog This pull request does not need a changelog entry label Feb 26, 2025
@jonludlam
Copy link
Member

Thanks @Julow !

@jonludlam jonludlam merged commit b8c8d99 into ocaml:master Feb 26, 2025
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This pull request does not need a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants