Skip to content

Improve argument checking & reduce strings to translate #6522

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Sep 25, 2022
Merged

Improve argument checking & reduce strings to translate #6522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
267ec1d
unify some 'must/should be an int' messages
jepler Jun 23, 2022
ef35ca1
vectorio: Simplify argument checking of x/y values
jepler Jun 23, 2022
f7221d5
simplify logic in _clobber_points_list
jepler Jun 23, 2022
18cb25e
Re-use another message
jepler Jun 23, 2022
8227903
Use validation functions
jepler Jun 23, 2022
4f75d09
Get rid of an unneeded message
jepler Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 1 addition & 23 deletions locale/circuitpython.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2526,8 +2526,7 @@ msgstr ""
msgid "can't cancel self"
msgstr ""

#: py/obj.c py/objint.c shared-bindings/i2ctarget/I2CTarget.c
#: shared-module/adafruit_pixelbuf/PixelBuf.c
#: py/obj.c py/objint.c shared-module/adafruit_pixelbuf/PixelBuf.c
msgid "can't convert %q to %q"
msgstr ""

Expand Down Expand Up @@ -2714,10 +2713,6 @@ msgstr ""
msgid "color must be between 0x000000 and 0xffffff"
msgstr ""

#: shared-bindings/displayio/ColorConverter.c
msgid "color should be an int"
msgstr ""

#: py/emitnative.c
msgid "comparison of int and uint"
msgstr ""
Expand Down Expand Up @@ -2869,10 +2864,6 @@ msgstr ""
msgid "end of format while looking for conversion specifier"
msgstr ""

#: shared-bindings/displayio/Shape.c
msgid "end_x should be an int"
msgstr ""

#: shared-bindings/alarm/time/TimeAlarm.c
msgid "epoch_time not supported on this board"
msgstr ""
Expand Down Expand Up @@ -3739,10 +3730,6 @@ msgstr ""
msgid "palette must be 32 bytes long"
msgstr ""

#: shared-bindings/displayio/Palette.c
msgid "palette_index should be an int"
msgstr ""

#: py/emitinlinextensa.c
msgid "parameters must be registers in sequence a2 to a5"
msgstr ""
Expand Down Expand Up @@ -3980,10 +3967,6 @@ msgstr ""
msgid "start/end indices"
msgstr ""

#: shared-bindings/displayio/Shape.c
msgid "start_x should be an int"
msgstr ""

#: shared-bindings/random/__init__.c
msgid "step must be non-zero"
msgstr ""
Expand Down Expand Up @@ -4198,7 +4181,6 @@ msgid "unreadable attribute"
msgstr ""

#: shared-bindings/displayio/TileGrid.c shared-bindings/vectorio/VectorShape.c
#: shared-module/vectorio/Polygon.c shared-module/vectorio/VectorShape.c
msgid "unsupported %q type"
msgstr ""

Expand Down Expand Up @@ -4328,10 +4310,6 @@ msgstr ""
msgid "xTaskCreate failed"
msgstr ""

#: shared-bindings/displayio/Shape.c
msgid "y should be an int"
msgstr ""

#: shared-module/displayio/Shape.c
msgid "y value out of bounds"
msgstr ""
Expand Down
2 changes: 1 addition & 1 deletion py/argcheck.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ mp_float_t mp_arg_validate_obj_float_non_negative(mp_obj_t float_in, mp_float_t
? default_for_null
: mp_obj_get_float(float_in);
if (f <= (mp_float_t)0.0) {
mp_raise_ValueError_varg(translate("%q must be >= 0"), arg_name);
mp_raise_ValueError_varg(translate("%q must be >= %d"), arg_name, 0);
}
return f;
}
Expand Down
6 changes: 2 additions & 4 deletions shared-bindings/audiobusio/PDMIn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,8 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(audiobusio_pdmin___exit___obj, 4, 4,
STATIC mp_obj_t audiobusio_pdmin_obj_record(mp_obj_t self_obj, mp_obj_t destination, mp_obj_t destination_length) {
audiobusio_pdmin_obj_t *self = MP_OBJ_TO_PTR(self_obj);
check_for_deinit(self);
if (!mp_obj_is_small_int(destination_length) || MP_OBJ_SMALL_INT_VALUE(destination_length) < 0) {
mp_raise_TypeError(translate("destination_length must be an int >= 0"));
}
uint32_t length = MP_OBJ_SMALL_INT_VALUE(destination_length);
uint32_t length = mp_arg_validate_type_int(destination_length, MP_QSTR_length);
mp_arg_validate_length_min(length, 0, MP_QSTR_length);

mp_buffer_info_t bufinfo;
if (mp_obj_is_type(destination, &mp_type_fileio)) {
Expand Down
5 changes: 1 addition & 4 deletions shared-bindings/displayio/ColorConverter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ STATIC mp_obj_t displayio_colorconverter_make_new(const mp_obj_type_t *type, siz
STATIC mp_obj_t displayio_colorconverter_obj_convert(mp_obj_t self_in, mp_obj_t color_obj) {
displayio_colorconverter_t *self = MP_OBJ_TO_PTR(self_in);

mp_int_t color;
if (!mp_obj_get_int_maybe(color_obj, &color)) {
mp_raise_ValueError(translate("color should be an int"));
}
mp_int_t color = mp_arg_validate_type_int(color_obj, MP_QSTR_color);
_displayio_colorspace_t colorspace;
colorspace.depth = 16;
uint32_t output_color;
Expand Down
17 changes: 4 additions & 13 deletions shared-bindings/displayio/Palette.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,8 @@ STATIC mp_obj_t palette_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t val
STATIC mp_obj_t displayio_palette_obj_make_transparent(mp_obj_t self_in, mp_obj_t palette_index_obj) {
displayio_palette_t *self = MP_OBJ_TO_PTR(self_in);

mp_int_t palette_index;
if (!mp_obj_get_int_maybe(palette_index_obj, &palette_index)) {
mp_raise_ValueError(translate("palette_index should be an int"));
}
palette_index = mp_arg_validate_int_range(palette_index, 0, common_hal_displayio_palette_get_len(self) - 1, MP_QSTR_palette_index);
mp_int_t palette_index = mp_arg_validate_type_int(palette_index_obj, MP_QSTR_palette_index);
mp_arg_validate_int_range(palette_index, 0, common_hal_displayio_palette_get_len(self) - 1, MP_QSTR_palette_index);

common_hal_displayio_palette_make_transparent(self, palette_index);
return mp_const_none;
Expand All @@ -173,10 +170,7 @@ MP_DEFINE_CONST_FUN_OBJ_2(displayio_palette_make_transparent_obj, displayio_pale
STATIC mp_obj_t displayio_palette_obj_make_opaque(mp_obj_t self_in, mp_obj_t palette_index_obj) {
displayio_palette_t *self = MP_OBJ_TO_PTR(self_in);

mp_int_t palette_index;
if (!mp_obj_get_int_maybe(palette_index_obj, &palette_index)) {
mp_raise_ValueError(translate("palette_index should be an int"));
}
mp_int_t palette_index = mp_arg_validate_type_int(palette_index_obj, MP_QSTR_palette_index);
palette_index = mp_arg_validate_int_range(palette_index, 0, common_hal_displayio_palette_get_len(self) - 1, MP_QSTR_palette_index);

common_hal_displayio_palette_make_opaque(self, palette_index);
Expand All @@ -191,10 +185,7 @@ MP_DEFINE_CONST_FUN_OBJ_2(displayio_palette_make_opaque_obj, displayio_palette_o
STATIC mp_obj_t displayio_palette_obj_is_transparent(mp_obj_t self_in, mp_obj_t palette_index_obj) {
displayio_palette_t *self = MP_OBJ_TO_PTR(self_in);

mp_int_t palette_index;
if (!mp_obj_get_int_maybe(palette_index_obj, &palette_index)) {
mp_raise_ValueError(translate("palette_index should be an int"));
}
mp_int_t palette_index = mp_arg_validate_type_int(palette_index_obj, MP_QSTR_palette_index);
palette_index = mp_arg_validate_int_range(palette_index, 0, common_hal_displayio_palette_get_len(self) - 1, MP_QSTR_palette_index);

return mp_obj_new_bool(common_hal_displayio_palette_is_transparent(self, palette_index));
Expand Down
15 changes: 3 additions & 12 deletions shared-bindings/displayio/Shape.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,9 @@ STATIC mp_obj_t displayio_shape_make_new(const mp_obj_type_t *type, size_t n_arg
STATIC mp_obj_t displayio_shape_obj_set_boundary(size_t n_args, const mp_obj_t *args) {
(void)n_args;
displayio_shape_t *self = MP_OBJ_TO_PTR(args[0]);
mp_int_t y;
if (!mp_obj_get_int_maybe(args[1], &y)) {
mp_raise_ValueError(translate("y should be an int"));
}
mp_int_t start_x;
if (!mp_obj_get_int_maybe(args[2], &start_x)) {
mp_raise_ValueError(translate("start_x should be an int"));
}
mp_int_t end_x;
if (!mp_obj_get_int_maybe(args[3], &end_x)) {
mp_raise_ValueError(translate("end_x should be an int"));
}
mp_int_t y = mp_arg_validate_type_int(args[1], MP_QSTR_y);
mp_int_t start_x = mp_arg_validate_type_int(args[1], MP_QSTR_start_x);
mp_int_t end_x = mp_arg_validate_type_int(args[1], MP_QSTR_end_x);
common_hal_displayio_shape_set_boundary(self, y, start_x, end_x);

return mp_const_none;
Expand Down
8 changes: 1 addition & 7 deletions shared-bindings/i2ctarget/I2CTarget.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,7 @@ STATIC mp_obj_t i2ctarget_i2c_target_make_new(const mp_obj_type_t *type, size_t
uint8_t *addresses = NULL;
unsigned int i = 0;
while ((item = mp_iternext(iterable)) != MP_OBJ_STOP_ITERATION) {
mp_int_t value;
if (!mp_obj_get_int_maybe(item, &value)) {
mp_raise_TypeError_varg(translate("can't convert %q to %q"), MP_QSTR_address, MP_QSTR_int);
}
if (value < 0x00 || value > 0x7f) {
mp_raise_ValueError(translate("address out of bounds"));
}
mp_uint_t value = mp_arg_validate_int_range(mp_obj_get_int(item), 0x00, 0x7f, MP_QSTR_address);
addresses = m_renew(uint8_t, addresses, i, i + 1);
addresses[i++] = value;
}
Expand Down
4 changes: 1 addition & 3 deletions shared-bindings/vectorio/Circle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ static mp_obj_t vectorio_circle_make_new(const mp_obj_type_t *type, size_t n_arg
mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);

mp_int_t radius = args[ARG_radius].u_int;
if (radius < 1) {
mp_raise_ValueError_varg(translate("%q must be >= 1"), MP_QSTR_radius);
}
mp_arg_validate_int_min(radius, 1, MP_QSTR_radius);

vectorio_circle_t *self = m_new_obj(vectorio_circle_t);
self->base.type = &vectorio_circle_type;
Expand Down
8 changes: 2 additions & 6 deletions shared-bindings/vectorio/Rectangle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,9 @@ static mp_obj_t vectorio_rectangle_make_new(const mp_obj_type_t *type, size_t n_
mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);

mp_int_t width = args[ARG_width].u_int;
if (width < 1) {
mp_raise_ValueError_varg(translate("%q must be >= 1"), MP_QSTR_width);
}
mp_arg_validate_int_min(width, 1, MP_QSTR_width);
mp_int_t height = args[ARG_height].u_int;
if (height < 1) {
mp_raise_ValueError_varg(translate("%q must be >= 1"), MP_QSTR_height);
}
mp_arg_validate_int_min(height, 1, MP_QSTR_height);

vectorio_rectangle_t *self = m_new_obj(vectorio_rectangle_t);
self->base.type = &vectorio_rectangle_type;
Expand Down
40 changes: 17 additions & 23 deletions shared-module/vectorio/Polygon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@


// Converts a list of points tuples to a flat list of ints for speedier internal use.
// Also validates the points.
// Also validates the points. If this fails due to invalid types or values, the
// number of points is 0 and the points_list is NULL.
static void _clobber_points_list(vectorio_polygon_t *self, mp_obj_t points_tuple_list) {
size_t len = 0;
mp_obj_t *items;
Expand All @@ -25,15 +26,12 @@ static void _clobber_points_list(vectorio_polygon_t *self, mp_obj_t points_tuple
mp_raise_TypeError(translate("Polygon needs at least 3 points"));
}

if (self->len < 2 * len) {
if (self->points_list != NULL) {
VECTORIO_POLYGON_DEBUG("free(%d), ", sizeof(self->points_list));
gc_free(self->points_list);
}
self->points_list = gc_alloc(2 * len * sizeof(uint16_t), false, false);
VECTORIO_POLYGON_DEBUG("alloc(%p, %d)", self->points_list, 2 * len * sizeof(uint16_t));
}
self->len = 2 * len;
int16_t *points_list = gc_realloc(self->points_list, 2 * len * sizeof(uint16_t), true);
VECTORIO_POLYGON_DEBUG("realloc(%p, %d) -> %p", self->points_list, 2 * len * sizeof(uint16_t), points_list);

// In case the validation calls below fail, set these values temporarily
self->points_list = NULL;
self->len = 0;

for (uint16_t i = 0; i < len; ++i) {
size_t tuple_len = 0;
Expand All @@ -42,20 +40,16 @@ static void _clobber_points_list(vectorio_polygon_t *self, mp_obj_t points_tuple

mp_arg_validate_length(tuple_len, 2, MP_QSTR_point);

mp_int_t x;
mp_int_t y;
if (!mp_obj_get_int_maybe(tuple_items[ 0 ], &x)
|| !mp_obj_get_int_maybe(tuple_items[ 1 ], &y)
|| x < SHRT_MIN || x > SHRT_MAX || y < SHRT_MIN || y > SHRT_MAX
) {
gc_free(self->points_list);
self->points_list = NULL;
mp_raise_ValueError_varg(translate("unsupported %q type"), MP_QSTR_point);
self->len = 0;
}
self->points_list[2 * i ] = (int16_t)x;
self->points_list[2 * i + 1] = (int16_t)y;
mp_int_t x = mp_arg_validate_type_int(tuple_items[0], MP_QSTR_x);
mp_arg_validate_int_range(x, SHRT_MIN, SHRT_MAX, MP_QSTR_x);
mp_int_t y = mp_arg_validate_type_int(tuple_items[1], MP_QSTR_y);
mp_arg_validate_int_range(y, SHRT_MIN, SHRT_MAX, MP_QSTR_y);
points_list[2 * i ] = (int16_t)x;
points_list[2 * i + 1] = (int16_t)y;
}

self->points_list = points_list;
self->len = 2 * len;
}


Expand Down
8 changes: 2 additions & 6 deletions shared-module/vectorio/VectorShape.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,8 @@ void common_hal_vectorio_vector_shape_set_location(vectorio_vector_shape_t *self
mp_obj_tuple_get(xy, &tuple_len, &tuple_items);
mp_arg_validate_length(tuple_len, 2, MP_QSTR_location);

mp_int_t x;
mp_int_t y;
if (!mp_obj_get_int_maybe(tuple_items[ 0 ], &x)
|| !mp_obj_get_int_maybe(tuple_items[ 1 ], &y)) {
mp_raise_ValueError_varg(translate("unsupported %q type"), MP_QSTR_point);
}
mp_int_t x = mp_arg_validate_type_int(tuple_items[0], MP_QSTR_x);
mp_int_t y = mp_arg_validate_type_int(tuple_items[0], MP_QSTR_y);
bool dirty = false;
if (self->x != x) {
check_bounds_and_set_x(self, x);
Expand Down