[PATCH evemu 2/2] python: encode/decode C strings for Python 3

Thu Nov 10 00:31:53 UTC 2016

On Wed, Nov 09, 2016 at 04:23:31PM +0100, Benjamin Tissoires wrote:
> On Wed, Nov 9, 2016 at 6:03 AM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > https://bugs.freedesktop.org/show_bug.cgi?id=97458
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  python/evemu/__init__.py | 29 +++++++++++++++++++++--------
> >  1 file changed, 21 insertions(+), 8 deletions(-)
> >
> > diff --git a/python/evemu/__init__.py b/python/evemu/__init__.py
> > index 230559f..323bcaf 100644
> > --- a/python/evemu/__init__.py
> > +++ b/python/evemu/__init__.py
> > @@ -50,20 +50,26 @@ def event_get_value(event_type, event_code = None):
> >
> >      if isinstance(event_type, int):
> >          event_type = _libevdev.libevdev_event_type_get_name(event_type)
> > -        if event_type == 0: # NULL
> > +        if event_type is None:
> 
> These changes seemed to be different than the ones mentioned in the commit.
> 0 and None is rather different and I'd like to be sure we are not
> screwing anything before applying this patch (which is why I applied
> only 1/2).

this one was a side effect. the == 0 check didn't work under python3, it
apparently returns None for a NULL pointer. But now I did some digging and
it turned out the previous check was bogus, it never worked under python2
either. It simply fell through until it triggered a correct condition
later. I'll send a patch out for that with a commit message to remember.

> 
> >              return None
> >
> > -    t = _libevdev.libevdev_event_type_from_name(str(event_type))
> > +        event_type = event_type.decode("iso8859-1")
> > +
> > +    event_type = str(event_type).encode("iso8859-1")
> 
> I had to run the python CLI to understand what was going on. But I am
> still not sure about the end result (the commit message would need to
> be extended):
> 
> When run under python2, _libevdev.libevdev_event_type_get_name()
> returns a plain "str" which can be reused immediately by
> _libevdev.libevdev_event_type_from_name().
> But when run under python3, _libevdev.libevdev_event_type_get_name()
> returns a bytes (b"SOMETHING"). But then
> _libevdev.libevdev_event_type_from_name() also requires a bytes string
> as argument.
> 
> So yes, I understand why there is a need to have the decode/encode
> dance (to actually have the type libevdev_event_type_from_name()
> expects).

fwiw, this is a general python3 thing, it's unicode all the way down with
the occasional turtle in between. You can't just take bytes anymore and cast
them, you need to decode them. Hence decode/encode game. AFAICT this is a
standard python thing, it's not specific to evemu as such.

> But why do we need to keep around the cast to a str? It looks like the
> original str() call was to convert the bytes string into a pure
> string. But the call was not working (returns something like
> "b'EV_KEY'"), so unless I am missing something, I'd rather drop this.

the original cast to str was because of the flexible API we have, not
because of the bytes, python2 doesn't care about those. These four calls are
equivalent:

   event_get_value("EV_ABS", "ABS_X")
   event_get_value(0x03, "ABS_X")
   event_get_value(0x03, 0x00)
   event_get_value("EV_ABS", 0x00)

iirc back when we implemented this, Daniel suggested the isinstance check
for int and simply to cast everything else to string and hope. So what is
passed in doesn't need to be a strict string, but casting should hopefully
make it one that works. This is a bit niche, I doubt anyone would pass an
object in with a __str__() that returns "EV_ABS" but hey, weirder things
have happened. either way, I just left this there for that reason.

> Also, how about we stick to string only (not bytes) for local
> variables (by calling t =
> _libevdev.libevdev_event_type_from_name(event_type.encode("iso8859-1")))?

I had that at first but it hides the encode/decode calls from view, making
it quite ugly:

        event_code = _libevdev.libevdev_event_code_get_name(t, str(event_code).encode("iso8859-1"))

is imo pretty unreadble

Cheers,
   Peter

> > +    t = _libevdev.libevdev_event_type_from_name(event_type)
> >
> >      if event_code is None:
> >          return None if t < 0 else t
> >
> >      if isinstance(event_code, int):
> >          event_code = _libevdev.libevdev_event_code_get_name(t, event_code)
> > -        if event_code == 0: # NULL
> > +        if event_code is None: # NULL
> >              return None
> >
> > -    c = _libevdev.libevdev_event_code_from_name(t, str(event_code))
> > +        event_code = event_code.decode("iso8859-1")
> > +
> > +    event_code = str(event_code).encode("iso8859-1")
> > +    c = _libevdev.libevdev_event_code_from_name(t, event_code)
> >
> >      return None if c < 0 else c
> >
> > @@ -84,7 +90,11 @@ def event_get_name(event_type, event_code = None):
> >
> >      if event_code is None:
> >          type_name = _libevdev.libevdev_event_type_get_name(event_type)
> > -        return None if type_name == 0 else type_name
> > +
> > +        if type_name is None:
> > +            return None
> > +
> > +        return type_name.decode("iso8859-1")
> >
> >      if not isinstance(event_code, int):
> >          event_code = event_get_value(event_type, event_code)
> > @@ -93,8 +103,10 @@ def event_get_name(event_type, event_code = None):
> >          return None
> >
> >      code_name = _libevdev.libevdev_event_code_get_name(event_type, event_code)
> > +    if code_name is None:
> > +        return None
> >
> > -    return None if code_name == 0 else code_name
> > +    return code_name.decode("iso8859-1")
> >
> >  def input_prop_get_name(prop):
> >      """
> > @@ -107,7 +119,7 @@ def input_prop_get_name(prop):
> >          return None
> >
> >      prop = _libevdev.libevdev_property_get_name(prop)
> > -    return None if prop == 0 else prop
> > +    return None if prop is None else prop.decode("iso8859-1")
> >
> >  def input_prop_get_value(prop):
> >      """
> > @@ -119,7 +131,8 @@ def input_prop_get_value(prop):
> >      if prop is None:
> >          return None
> >
> > -    prop = _libevdev.libevdev_property_from_name(str(prop))
> > +    prop = str(prop).encode("iso8859-1")
> > +    prop = _libevdev.libevdev_property_from_name(prop)
> >      return None if prop < 0 else prop
> >
> >  class InputEvent(object):
> > --
> > 2.9.3
> >
> > _______________________________________________
> > Input-tools mailing list
> > Input-tools at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/input-tools