Re: [libgpiod] Bug in python binding when requesting output?

[Kent Gibson <warthog618@xxxxxxxxx>]

On Thu, Feb 11, 2021 at 09:54:22PM +0100, Pedro Botella wrote:
> Hi,
> 
> I'm experiencing what I think is a bug in the python bindings for libgpiod.
> I believe a line.request with type gpiod.LINE_REQ_DIR_OUT always
> results in that line being set to '0'.

That is correct - when requesting a line as output at the kernel uAPI
the initial value must always be provided.  If you do not provide
default_vals via the Python API then the output should be defaulted to
'0' by the Python binding.

> To reproduce:
> 1. request a line with type gpiod.LINE_REQ_DIR_OUT
> 2. set the line to '1'
> 3. release the line
> 4. request the same line with type gpiod.LINE_REQ_DIR_OUT
> 5. get the value, it should now be '0'
> 

To clarify, the expected behaviour is that the output is defaulted
to '0' if default values are not provided.
So the problem you are seeing is that the output is not consistently '0'?

If you are expecting to see a '1' then you are expecting the lack of
default_vals in the kwds to leave the output value as is, but that is
not the case - it should default to '0'.

> I think the issue is in "gpiod_LineBulk_request" in
> https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/bindings/python/gpiodmodule.c
> There a call to "gpiod_line_request_bulk" with default_vals being
> passed as a pointer. Later on in the code, this parameter is checked
> for NULL, if it is not NULL then the values in the array are used as
> default_vals.
> I believe that a NULL pointer should be passed instead if no
> default_vals have been requested when doing a Line.request from
> Python.
> 

Agreed - passing default_vals uninitialized to gpiod_line_request_bulk()
is a bug.
It should be zeroed, or a NULL pointer should be passed if the
default_vals were not provided in the kwds. Otherwise the output
value will be set based on the uninitializezd contents of default_vals.

Would you like to provide a patch?

In the meantime the obvious workaround is to always provide default_vals
in the kwds.

Cheers,
Kent.