Skip to content
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

Truncating buffers could come in handy #6975

Closed
vicuna opened this issue Aug 30, 2015 · 4 comments
Closed

Truncating buffers could come in handy #6975

vicuna opened this issue Aug 30, 2015 · 4 comments

Comments

@vicuna
Copy link

vicuna commented Aug 30, 2015

Original bug ID: 6975
Reporter: omion
Assigned to: @alainfrisch
Status: resolved (set by @alainfrisch on 2016-12-06T15:26:39Z)
Resolution: fixed
Priority: low
Severity: feature
Version: 4.02.0
Fixed in version: 4.05.0 +dev/beta1/beta2/beta3/rc1
Category: standard library
Tags: junior_job
Related to: #3774 #6538
Monitored by: @diml @hcarty

Bug description

I've been writing some code that incrementally adds data to a buffer, but will fail if it gets too long. However, exactly how long the data will be would be difficult to know in advance, so I just have to start writing and give up if it gets too long. I'd like to be able to clear out my half-written data when that happens, but it looks like the Buffer module doesn't really allow that.

Essentially, I'm doing:

let try_write buf data max_chars =
let start_len = Buffer.length buf in
write_lots_of_stuff buf data;
if Buffer.length buf > start_len + max_chars then (
(* Too long )
Buffer.truncate buf start_len;
false
) else (
(
OK length *)
true
)

Buffer.truncate would be trivial to implement in O(1) with access to the Buffer.t internals, but would be difficult and inefficient without.

I was thinking of something like this:

let truncate b len =
if len < 0 || len > b.position
then invalid_arg "Buffer.truncate"
else b.position <- len

@vicuna
Copy link
Author

vicuna commented Aug 30, 2015

Comment author: @alainfrisch

I'm in favor of the change: it's useful, easy to implement (and impossible outside the implementation with the same performance profile) and doesn't break invariants of the data structure (one could already reset the buffer and re-inject a prefix of the previous content).

Another useful addition would be to allow extracting a substring of the buffer's content.

As a matter of fact, it would be worth turning Buffer into a full-fledged implementation of "mutable and extensible strings". This would include blitting (from strings, bytes, Buffers) into arbitrary segments of a Buffer.t.

@vicuna
Copy link
Author

vicuna commented Nov 27, 2015

Comment author: @damiendoligez

I'm also in favor for truncate and substring extraction.

Blitting into buffers is more tricky.

@vicuna
Copy link
Author

vicuna commented Aug 22, 2016

Comment author: maro

I would also really like truncate.

I see however at least two things that could lead to further discussions:

  1. Should truncate accept arguments larger than the current buffer length? I'd say yes to that, to avoid having to check before calling truncate, e.g.:

let safe_append_stuff buf s =
Buffer.add_string buf s;
Buffer.truncate buf max_len

Instead of having to do something as:

let safe_append_stuff buf s =
Buffer.add_string buf s;
if Buffer.length buf > max_len then Buffer.truncate buf max_len

But that's a minor detail, honestly both would be fine for me.

  1. Should the internal length of the Bytes buffer be modified? I'd say no; that may waste some space, but it would slow down truncate. However, I don't know if this has unintended consequences.

Considering these aspects, one implementation of truncate might be as simple as:

(** [truncate b n] truncates the length of [b] to be no larger than [n].
Does nothing if the length of [b] is already smaller than or equal to [n].
Note that it does not change the size of the underlying buffer.
Raise [Invalid_argument] if [n < 0]. *)
let truncate b n =
if n < 0 then invalid_arg "Buffer.truncate"
else if b.position > n then b.position <- n

Is it worth creating a Github PR for that, or is it too small of a change?

@vicuna
Copy link
Author

vicuna commented Dec 6, 2016

Comment author: @alainfrisch

#902

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants