Skip to content

Audit and fix native API for methods that accept and ignore extra args. #321

@tannewt

Description

@tannewt

Python methods can take two type of arguments:

  • Positional args that depend on their position in the list and must be given.
  • Keyword args are optional and either depend on position or can be identified by their name.

For example, audioio.AudioOut takes two positional arguments, pin and sample_source and no keyword arguments.

In contrast, AudioOut.play takes one keyword argument loop that defaults to False and can be positionally specified (a.play(True)) or specified by name (a.play(loop=True)).

There is a common bug in CircuitPython related to arguments where some methods defined in shared-bindings accept extra random arguments (both positional and keyword) without a fuss (raising TypeError: extra keyword arguments given or TypeError: foo() takes at most # arguments). This can lead to great confusion! Normally Python will check argument validity for us but when we implement Python using C it trusts us to check.

There are a couple reasons this happens that need to be checked and fixed:

Constructors (*make_new in C and equivalent to __init__ in Python), like audioio.AudioOut, should use mp_arg_check_num to valid the number of arguments they receive. The last argument to mp_arg_check_num indicates whether keyword arguments should be accepted. This is often wrong and should be fixed. The acceptable number of positional arguments (args 3 and 4) can also be wrong!

Method arguments can be checked when methods are declared using a macro that can restrict the number of arguments such as:

These macros can be misused and unintentionally allow more arguments than those used. Picking the best macro for each function is the first step to getting method argument checking correct. If the macro is wrong, the method may not catch extra positional arguments.

So, the task is to look through shared-bindings and validate that the argument checking matches the arguments in the docs. If it doesn't, then fix it!

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions