8000 py/modio: Handle partial writes on BufferedWriter · micropython/micropython@321f045 · GitHub
[go: up one dir, main page]

Skip to content

Commit 321f045

Browse files
committed
py/modio: Handle partial writes on BufferedWriter
To simplify the logic create bufwriter_do_write. This function keeps track of the data left on self->len and, if a partial write happens, this function uses memmove to move the remaining data back to the beginning of the buffer. This allows simplifying bufwriter_write and bufwriter_flush significantly. bufwriter_flush now only needs to call this function if the buffer has some data stored and check for any error codes. Additionally, if the buffer is only partially flushed we notify the user too (so that they know there might be data left). bufwriter_write now just needs to call this function whenever the buffer gets full and copy the input into the buffer. It will return when either no data is written at all or when all of the input is consumed. In the case of a partial write it returns exactly the amount of data which was written. Additionally allow caching of errors to better handle partial writes. Until now if an error occurred during the write, the error would be raised and the caller had no way to know if any data was written at all (for example in prior calls if more than one block of data was passed as input). Now when we have written out some data and an error happens, we reset the buffer to the state it would have if it did not contain the data that was not written (and which was not buffered previously), and then, we return the data that was written (if any) or raise an error if no data from the input was written. This allows the programmer better control of writes. In particular, the programmer will know exactly how much of its last input data was written, consequently allowing it to handle whatever data left to be written in a better way. Signed-off-by: Francisco Blas (klondike) Izquierdo Riera <klondike@klondike.es>
1 parent 2d1e8f7 commit 321f045

File tree

1 file changed

+68
-30
lines changed

1 file changed

+68
-30
lines changed

py/modio.c

Lines changed: 68 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <assert.h>
2828
#include <string.h>
2929

30+
#include "py/misc.h"
3031
#include "py/runtime.h"
3132
#include "py/builtin.h"
3233
#include "py/stream.h"
@@ -113,6 +114,7 @@ typedef struct _mp_obj_bufwriter_t {
113114
mp_obj_t stream;
114115
size_t alloc;
115116
size_t len;
117+
int error;
116118
byte buf[0];
117119
} mp_obj_bufwriter_t;
118120

@@ -123,61 +125,97 @@ STATIC mp_obj_t bufwriter_make_new(const mp_obj_type_t *type, size_t n_args, siz
123125
o->stream = args[0];
124126
o->alloc = alloc;
125127
o->len = 0;
128+
o->error = 0;
126129
return o;
127130
}
128131

132+
// Writes out the data stored in the buffer so far
133+
STATIC int bufwriter_do_write(mp_obj_bufwriter_t *self) {
134+
int rv = 0;
135+
// This cannot return 0 without an error
136+
mp_uint_t out_sz = mp_stream_write_exactly(self->stream, self->buf, self->len, &rv);
137+
self->len -= out_sz;
138+
// Copy the non written characters back to the beginning
139+
if (self->len != 0) {
140+
// Use memmove since there might be overlaps
141+
memmove(self->buf, self->buf + out_sz, self->len);
142+
}
143+
return rv;
144+
}
145+
129146
STATIC mp_uint_t bufwriter_write(mp_obj_t self_in, const void *buf, mp_uint_t size, int *errcode) {
130147
mp_obj_bufwriter_t *self = MP_OBJ_TO_PTR(self_in);
131148

132149
mp_uint_t org_size = size;
150+
// Alloc should always remain the same so cache it.
151+
size_t alloc = self->alloc;
152+
mp_uint_t rem = 0; // No data has been copied from the buffer so far
133153

134-
while (size > 0) {
135-
mp_uint_t rem = self->alloc - self->len;
136-
if (size < rem) {
137-
memcpy(self->buf + self->len, buf, size);
138-
self->len += size;
139-
return org_size;
140-
}
154+
if (self->error != 0) {
155+
*errcode = self->error;
156+
self->error = 0;
157+
return MP_STREAM_ERROR;
158+
}
141159

160+
// Using this form allows ensuring that we try to empty the buffer if it is
161+
// full without trying to extract any data first.
162+
while (true) {
142163
// Buffer flushing policy here is to flush entire buffer all the time.
143164
// This allows e.g. to have a block device as backing storage and write
144165
// entire block to it. memcpy below is not ideal and could be optimized
145166
// in some cases. But the way it is now it at least ensures that buffer
146167
// is word-aligned, to guard against obscure cases when it matters, e.g.
147168
// https://github.com/micropython/micropython/issues/1863
169+
// Try to empty the buffer first
170+
if (self->len == alloc) {
171+
if ((*errcode = bufwriter_do_write(self)) != 0) {
172+
// If this is the first write with data from the new buffer
173+
// But no data from the new buffer was written out,
174+
// then remove the data added from the new buffer and raise
175+
// the error
176+
if (org_size == size + rem && self->len >= rem) {
177+
// Remove the extra non-written data from the buffer and error
178+
self->len -= rem;
179+
return MP_STREAM_ERROR;
180+
}
181+
// Some data from the new buffer has been written rollback as much
182+
// as possible and return what was written then raise an error
183+
// on the next call.
184+
size += self->len;
185+
self->len = 0;
186+
self->error = *errcode;
187+
*errcode = 0;
188+
return org_size - size;
189+
}
190+
}
191+
// No data left to write
192+
if (size == 0) {
193+
return org_size;
194+
}
195+
196+
rem = MIN(alloc - self->len, size);
148197
memcpy(self->buf + self->len, buf, rem);
198+
self->len += rem;
149199
buf = (byte *)buf + rem;
150200
size -= rem;
151-
mp_uint_t out_sz = mp_stream_write_exactly(self->stream, self->buf, self->alloc, errcode);
152-
(void)out_sz;
153-
if (*errcode != 0) {
154-
return MP_STREAM_ERROR;
155-
}
156-
// TODO: try to recover from a case of non-blocking stream, e.g. move
157-
// remaining chunk to the beginning of buffer.
158-
assert(out_sz == self->alloc);
159-
self->len = 0;
160201
}
161-
162-
return org_size;
163202
}
164203

165204
STATIC mp_obj_t bufwriter_flush(mp_obj_t self_in) {
166205
mp_obj_bufwriter_t *self = MP_OBJ_TO_PTR(self_in);
167-
168-
if (self->len != 0) {
169-
int err;
170-
mp_uint_t out_sz = mp_stream_write_exactly(self->stream, self->buf, self->len, &err);
171-
(void)out_sz;
172-
// TODO: try to recover from a case of non-blocking stream, e.g. move
173-
// remaining chunk to the beginning of buffer.
174-
assert(out_sz == self->len);
175-
self->len = 0;
176-
if (err != 0) {
177-
mp_raise_OSError(err);
206+
int err = self->error;
207+
if (err == 0 && self->len != 0) {
208+
err = bufwriter_do_write(self);
209+
// If we couldn't flush the whole buffer notify the user.
210+
if (err == 0 && self->len != 0) {
211+
err = MP_EAGAIN;
178212
}
179213
}
180-
214+
// If there is an error raise it
215+
if (err != 0) {
216+
self->error = 0;
217+
mp_raise_OSError(err);
218+
}
181219
return mp_const_none;
182220
}
183221
STATIC MP_DEFINE_CONST_FUN_OBJ_1(bufwriter_flush_obj, bufwriter_flush);

0 commit comments

Comments
 (0)
0