[PATCH] tevent: Do not use uninitialized tv in py_tevent_context_add_timer

[Douglas Bagnall]

[Resending, because I used the wrong from: address the first time]

On 19/01/19 4:52 AM, Lukas Slebodnik via samba-technical wrote:
> ehlo,
> 
> I could see valgrind error in python bindings
> 
> LS
> 

Yes. Good, almost.

We are trying to switch to C99 initialisers
(c.f. https://gitlab.com/samba-team/samba/merge_requests/198), and it would be
even better if you could initialise callback to NULL.

> --- a/lib/tevent/pytevent.c
> +++ b/lib/tevent/pytevent.c
> @@ -476,7 +476,7 @@ static PyObject *py_tevent_context_add_timer_internal(TeventContext_Object *self
>  
>  static PyObject *py_tevent_context_add_timer(TeventContext_Object *self, PyObject *args)
>  {
> -	struct timeval next_event;
> +	struct timeval next_event = { 0 };
>  	PyObject *callback;
>  	if (!PyArg_ParseTuple(args, "lO", &next_event, &callback))
>  		return NULL;

BUT the underlying question is where is used uninitialised?
PyArg_ParseTuple() promises to initialise it or return NULL. Except it is
asked to initialise a signed long ("l" in the format string), while
struct timeval is two numbers (seemingly always longs), so this is just
initialising the tv_sec, not the tv_usec:

      PyArg_ParseTuple(args, "lO", &next_event, &callback)


Anyway, if you can C99 it, RB+ from me, but this needs more looking at, probably
with the python argument converted to a double, then mangled into secs/usecs
in C.

Looking at this:

$ git grep add_timer :/*.py
lib/tevent/bindings.py:        timer = self.ctx.add_timer(0, lambda t: collecting_list.append(True))
lib/tevent/bindings.py:        timer = self.ctx.add_timer(0, lambda t: collecting_list.append(True))
lib/tevent/bindings.py:        timer = self.ctx.add_timer(0, lambda t: collecting_list.append(True))
lib/tevent/bindings.py:        self.ctx.add_timer_offset(0.2, lambda t: collecting_list.append(2))
lib/tevent/bindings.py:        self.ctx.add_timer_offset(0.1, lambda t: collecting_list.append(1))

With this patch those last two won't work as expected (rounded to 0 int).
Currently NONE of them would work as expected, resulting in 0 secs + $RAND usecs.

It is possible you have just discovered why we have an infinite supply of timing
related flapping tests, though this is not the first time I have thought that.


cheers,
Douglas