Skip to content

Commit 0d3cb67

Browse files
dpgeorgedpgeorge
committed
py: Change vstr so that it doesn't null terminate buffer by default.
This cleans up vstr so that it's a pure "variable buffer", and the user can decide whether they need to add a terminating null byte. In most places where vstr is used, the vstr did not need to be null terminated and so this patch saves code size, a tiny bit of RAM, and makes vstr usage more efficient. When null termination is needed it must be done explicitly using vstr_null_terminate.
1 parent 57aebe1 commit 0d3cb67

File tree

14 files changed

+67
-70
lines changed

14 files changed

+67
-70
lines changed

‎extmod/modujson.c‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ STATIC mp_obj_t mod_ujson_dumps(mp_obj_t obj) {
3939
vstr_t vstr;
4040
vstr_init(&vstr, 8);
4141
mp_obj_print_helper((void (*)(void *env, const char *fmt, ...))vstr_printf, &vstr, obj, PRINT_JSON);
42-
mp_obj_t ret = mp_obj_new_str(vstr.buf, vstr.len, false);
43-
vstr_clear(&vstr);
44-
return ret;
42+
return mp_obj_new_str_from_vstr(&mp_type_str, &vstr);
4543
}
4644
STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_ujson_dumps_obj, mod_ujson_dumps);
4745

‎lib/mp-readline/readline.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ int readline_process_char(int c) {
111111
if (rl.line->len > rl.orig_line_len && (MP_STATE_PORT(readline_hist)[0] == NULL || strcmp(MP_STATE_PORT(readline_hist)[0], rl.line->buf + rl.orig_line_len) != 0)) {
112112
// a line which is not empty and different from the last one
113113
// so update the history
114+
vstr_null_terminate(rl.line);
114115
char *most_recent_hist = str_dup_maybe(rl.line->buf + rl.orig_line_len);
115116
if (most_recent_hist != NULL) {
116117
for (int i = READLINE_HIST_SIZE - 1; i > 0; i--) {

‎py/builtinimport.c‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,14 @@ bool mp_obj_is_package(mp_obj_t module) {
6161
}
6262

6363
STATIC mp_import_stat_t stat_dir_or_file(vstr_t *path) {
64+
vstr_null_terminate(path);
6465
//printf("stat %s\n", vstr_str(path));
6566
mp_import_stat_t stat = mp_import_stat(vstr_str(path));
6667
if (stat == MP_IMPORT_STAT_DIR) {
6768
return stat;
6869
}
6970
vstr_add_str(path, ".py");
71+
vstr_null_terminate(path);
7072
stat = mp_import_stat(vstr_str(path));
7173
if (stat == MP_IMPORT_STAT_FILE) {
7274
return stat;
@@ -134,6 +136,7 @@ STATIC void do_load_from_lexer(mp_obj_t module_obj, mp_lexer_t *lex, const char
134136

135137
STATIC void do_load(mp_obj_t module_obj, vstr_t *file) {
136138
// create the lexer
139+
vstr_null_terminate(file);
137140
mp_lexer_t *lex = mp_lexer_new_from_file(vstr_str(file));
138141
do_load_from_lexer(module_obj, lex, vstr_str(file));
139142
}
@@ -263,7 +266,7 @@ mp_obj_t mp_builtin___import__(mp_uint_t n_args, const mp_obj_t *args) {
263266
// create a qstr for the module name up to this depth
264267
qstr mod_name = qstr_from_strn(mod_str, i);
265268
DEBUG_printf("Processing module: %s\n", qstr_str(mod_name));
266-
DEBUG_printf("Previous path: %s\n", vstr_str(&path));
269+
DEBUG_printf("Previous path: %.*s\n", vstr_len(&path), vstr_str(&path));
267270

268271
// find the file corresponding to the module name
269272
mp_import_stat_t stat;
@@ -276,7 +279,7 @@ mp_obj_t mp_builtin___import__(mp_uint_t n_args, const mp_obj_t *args) {
276279
vstr_add_strn(&path, mod_str + last, i - last);
277280
stat = stat_dir_or_file(&path);
278281
}
279-
DEBUG_printf("Current path: %s\n", vstr_str(&path));
282+
DEBUG_printf("Current path: %.*s\n", vstr_len(&path), vstr_str(&path));
280283

281284
if (stat == MP_IMPORT_STAT_NO_EXIST) {
282285
#if MICROPY_MODULE_WEAK_LINKS
@@ -320,12 +323,13 @@ mp_obj_t mp_builtin___import__(mp_uint_t n_args, const mp_obj_t *args) {
320323
}
321324

322325
if (stat == MP_IMPORT_STAT_DIR) {
323-
DEBUG_printf("%s is dir\n", vstr_str(&path));
326+
DEBUG_printf("%.*s is dir\n", vstr_len(&path), vstr_str(&path));
324327
// https://docs.python.org/3/reference/import.html
325328
// "Specifically, any module that contains a __path__ attribute is considered a package."
326329
mp_store_attr(module_obj, MP_QSTR___path__, mp_obj_new_str(vstr_str(&path), vstr_len(&path), false));
327330
vstr_add_char(&path, PATH_SEP_CHAR);
328331
vstr_add_str(&path, "__init__.py");
332+
vstr_null_terminate(&path);
329333
if (mp_import_stat(vstr_str(&path)) != MP_IMPORT_STAT_FILE) {
330334
vstr_cut_tail_bytes(&path, sizeof("/__init__.py") - 1); // cut off /__init__.py
331335
mp_warning("%s is imported as namespace package", vstr_str(&path));

‎py/compile.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ STATIC void cpython_c_tuple(compiler_t *comp, mp_parse_node_t pn, mp_parse_node_
555555
} else {
556556
vstr_printf(vstr, ")");
557557
}
558-
EMIT_ARG(load_const_verbatim_str, vstr_str(vstr));
558+
EMIT_ARG(load_const_verbatim_strn, vstr_str(vstr), vstr_len(vstr));
559559
vstr_free(vstr);
560560
} else {
561561
if (!MP_PARSE_NODE_IS_NULL(pn)) {
@@ -1538,7 +1538,7 @@ STATIC void compile_import_from(compiler_t *comp, mp_parse_node_struct_t *pns) {
15381538

15391539
// build the "fromlist" tuple
15401540
#if MICROPY_EMIT_CPYTHON
1541-
EMIT_ARG(load_const_verbatim_str, "('*',)");
1541+
EMIT_ARG(load_const_verbatim_strn, "('*',)", 6);
15421542
#else
15431543
EMIT_ARG(load_const_str, MP_QSTR__star_, false);
15441544
EMIT_ARG(build_tuple, 1);
@@ -1576,7 +1576,7 @@ STATIC void compile_import_from(compiler_t *comp, mp_parse_node_struct_t *pns) {
15761576
vstr_printf(vstr, ",");
15771577
}
15781578
vstr_printf(vstr, ")");
1579-
EMIT_ARG(load_const_verbatim_str, vstr_str(vstr));
1579+
EMIT_ARG(load_const_verbatim_strn, vstr_str(vstr), vstr_len(vstr));
15801580
vstr_free(vstr);
15811581
}
15821582
#else

‎py/emit.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ typedef struct _emit_method_table_t {
157157

158158
#if MICROPY_EMIT_CPYTHON
159159
// these methods are only needed for emitcpy
160-
void (*load_const_verbatim_str)(emit_t *emit, const char *str);
160+
void (*load_const_verbatim_strn)(emit_t *emit, const char *str, mp_uint_t len);
161161
void (*load_closure)(emit_t *emit, qstr qst, mp_uint_t local_num);
162162
void (*setup_loop)(emit_t *emit, mp_uint_t label);
163163
#endif

‎py/emitcpy.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -800,10 +800,10 @@ STATIC void emit_cpy_end_except_handler(emit_t *emit) {
800800
emit_cpy_adjust_stack_size(emit, -5); // stack adjust
801801
}
802802

803-
STATIC void emit_cpy_load_const_verbatim_str(emit_t *emit, const char *str) {
803+
STATIC void emit_cpy_load_const_verbatim_strn(emit_t *emit, const char *str, mp_uint_t len) {
804804
emit_pre(emit, 1, 3);
805805
if (emit->pass == MP_PASS_EMIT) {
806-
printf("LOAD_CONST %s\n", str);
806+
printf("LOAD_CONST %.*s\n", (int)len, str);
807807
}
808808
}
809809

@@ -912,7 +912,7 @@ const emit_method_table_t emit_cpython_method_table = {
912912
emit_cpy_end_except_handler,
913913

914914
// emitcpy specific functions
915-
emit_cpy_load_const_verbatim_str,
915+
emit_cpy_load_const_verbatim_strn,
916916
emit_cpy_load_closure,
917917
emit_cpy_setup_loop,
918918
};

‎py/misc.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ size_t vstr_len(vstr_t *vstr);
145145
void vstr_hint_size(vstr_t *vstr, size_t size);
146146
char *vstr_extend(vstr_t *vstr, size_t size);
147147
char *vstr_add_len(vstr_t *vstr, size_t len);
148+
void vstr_null_terminate(vstr_t *vstr);
148149
void vstr_add_byte(vstr_t *vstr, byte v);
149150
void vstr_add_char(vstr_t *vstr, unichar chr);
150151
void vstr_add_str(vstr_t *vstr, const char *str);

‎py/objstr.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ STATIC mp_obj_t bytes_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_k
217217
vstr_init(&vstr, 16);
218218
} else {
219219
mp_int_t len = MP_OBJ_SMALL_INT_VALUE(len_in);
220-
vstr_init(&vstr, len + 1);
220+
vstr_init(&vstr, len);
221221
}
222222

223223
mp_obj_t iterable = mp_getiter(args[0]);
@@ -856,7 +856,7 @@ mp_obj_t mp_obj_str_format(mp_uint_t n_args, const mp_obj_t *args, mp_map_t *kwa
856856
while (str < top && *str != '}' && *str != '!' && *str != ':') {
857857
vstr_add_char(field_name, *str++);
858858
}
859-
vstr_add_char(field_name, '\0');
859+
vstr_null_terminate(field_name);
860860
}
861861

862862
// conversion ::= "r" | "s"
@@ -887,7 +887,7 @@ mp_obj_t mp_obj_str_format(mp_uint_t n_args, const mp_obj_t *args, mp_map_t *kwa
887887
while (str < top && *str != '}') {
888888
vstr_add_char(format_spec, *str++);
889889
}
890-
vstr_add_char(format_spec, '\0');
890+
vstr_null_terminate(format_spec);
891891
}
892892
}
893893
if (str >= top) {
@@ -1890,6 +1890,7 @@ mp_obj_t mp_obj_new_str_from_vstr(const mp_obj_type_t *type, vstr_t *vstr) {
18901890
o->len = vstr->len;
18911891
o->hash = qstr_compute_hash((byte*)vstr->buf, vstr->len);
18921892
o->data = (byte*)m_renew(char, vstr->buf, vstr->alloc, vstr->len + 1);
1893+
((byte*)o->data)[o->len] = '\0'; // add null byte
18931894
vstr->buf = NULL;
18941895
vstr->alloc = 0;
18951896
return o;

‎py/stream.c‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ STATIC mp_obj_t stream_read(mp_uint_t n_args, const mp_obj_t *args) {
177177
nlr_raise(mp_obj_new_exception_arg1(&mp_type_OSError, MP_OBJ_NEW_SMALL_INT(error)));
178178
} else {
179179
vstr.len = out_sz;
180-
vstr.buf[vstr.len] = '\0';
181180
return mp_obj_new_str_from_vstr(STREAM_CONTENT_TYPE(o->type->stream_p), &vstr);
182181
}
183182
}
@@ -289,7 +288,6 @@ STATIC mp_obj_t stream_readall(mp_obj_t self_in) {
289288
}
290289

291290
vstr.len = total_size;
292-
vstr.buf[total_size] = '\0';
293291
return mp_obj_new_str_from_vstr(STREAM_CONTENT_TYPE(o->type->stream_p), &vstr);
294292
}
295293

@@ -306,15 +304,15 @@ STATIC mp_obj_t stream_unbuffered_readline(mp_uint_t n_args, const mp_obj_t *arg
306304
max_size = MP_OBJ_SMALL_INT_VALUE(args[1]);
307305
}
308306

309-
vstr_t *vstr;
307+
vstr_t vstr;
310308
if (max_size != -1) {
311-
vstr = vstr_new_size(max_size);
309+
vstr_init(&vstr, max_size);
312310
} else {
313-
vstr = vstr_new();
311+
vstr_init(&vstr, 16);
314312
}
315313

316314
while (max_size == -1 || max_size-- != 0) {
317-
char *p = vstr_add_len(vstr, 1);
315+
char *p = vstr_add_len(&vstr, 1);
318316
if (p == NULL) {
319317
nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_MemoryError, "out of memory"));
320318
}
@@ -323,14 +321,14 @@ STATIC mp_obj_t stream_unbuffered_readline(mp_uint_t n_args, const mp_obj_t *arg
323321
mp_uint_t out_sz = o->type->stream_p->read(o, p, 1, &error);
324322
if (out_sz == MP_STREAM_ERROR) {
325323
if (is_nonblocking_error(error)) {
326-
if (vstr->len == 1) {
324+
if (vstr.len == 1) {
327325
// We just incremented it, but otherwise we read nothing
328326
// and immediately got EAGAIN. This is case is not well
329327
// specified in
330328
// https://docs.python.org/3/library/io.html#io.IOBase.readline
331329
// unlike similar case for read(). But we follow the latter's
332330
// behavior - return None.
333-
vstr_free(vstr);
331+
vstr_clear(&vstr);
334332
return mp_const_none;
335333
} else {
336334
goto done;
@@ -343,16 +341,15 @@ STATIC mp_obj_t stream_unbuffered_readline(mp_uint_t n_args, const mp_obj_t *arg
343341
// Back out previously added byte
344342
// Consider, what's better - read a char and get OutOfMemory (so read
345343
// char is lost), or allocate first as we do.
346-
vstr_cut_tail_bytes(vstr, 1);
344+
vstr_cut_tail_bytes(&vstr, 1);
347345
break;
348346
}
349347
if (*p == '\n') {
350348
break;
351349
}
352350
}
353-
mp_obj_t ret = mp_obj_new_str_from_vstr(STREAM_CONTENT_TYPE(o->type->stream_p), vstr);
354-
vstr_free(vstr);
355-
return ret;
351+
352+
return mp_obj_new_str_from_vstr(STREAM_CONTENT_TYPE(o->type->stream_p), &vstr);
356353
}
357354

358355
// TODO take an optional extra argument (what does it do exactly?)

0 commit comments

Comments
 (0)