[PATCH libevdev 9/9] test: test for logging function

[Benjamin Tissoires]

On Tue, Sep 10, 2013 at 1:54 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Mon, Sep 09, 2013 at 05:30:29PM +0200, David Herrmann wrote:
>> Hi Benjamin
>>
>> On Mon, Sep 9, 2013 at 5:09 PM, Benjamin Tissoires
>> <benjamin.tissoires at gmail.com> wrote:
>> > On Tue, Sep 3, 2013 at 9:07 AM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
>> >> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>> >> ---
>> >>  test/test-libevdev-init.c | 25 +++++++++++++++++--------
>> >>  1 file changed, 17 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/test/test-libevdev-init.c b/test/test-libevdev-init.c
>> >> index 8f1d0d1..2610975 100644
>> >> --- a/test/test-libevdev-init.c
>> >> +++ b/test/test-libevdev-init.c
>> >> @@ -93,10 +93,16 @@ START_TEST(test_init_and_change_fd)
>> >>  }
>> >>  END_TEST
>> >>
>> >> +
>> >> +static int log_fn_called = 0;
>> >> +static char *logdata = "test";
>> >>  static void logfunc(enum libevdev_log_priority priority,
>> >>                     void *data,
>> >>                     const char *file, int line, const char *func,
>> >> -                   const char *f, va_list args) {}
>> >> +                   const char *f, va_list args) {
>> >> +       ck_assert_int_eq(strcmp(logdata, data), 0);
>> >> +       log_fn_called++;
>> >> +}
>> >>
>> >>  START_TEST(test_log_init)
>> >>  {
>> >> @@ -107,16 +113,19 @@ START_TEST(test_log_init)
>> >>
>> >>         dev = libevdev_new();
>> >>         ck_assert(dev != NULL);
>> >> -       libevdev_set_log_function(logfunc, NULL);
>> >> -       libevdev_free(dev);
>> >>
>> >> -       dev = libevdev_new();
>> >> -       ck_assert(dev != NULL);
>> >> +       libevdev_set_log_function(logfunc, logdata);
>> >> +       libevdev_next_event(dev, LIBEVDEV_READ_NORMAL, NULL);
>> >> +
>> >>         libevdev_set_log_function(NULL, NULL);
>> >> -       libevdev_set_log_function(logfunc, NULL);
>> >> +       libevdev_next_event(dev, LIBEVDEV_READ_NORMAL, NULL);
>> >> +
>> >> +       libevdev_set_log_function(logfunc, logdata);
>> >
>> > Shoudn't this be "libevdev_set_log_function(logfunc, NULL);"?
>> > Otherwise, I don't get the "2" below.
>>
>> You would get a NULL-deref in strcmp() in logfunc() if it's NULL. I
>> think it looks all good. He tests _next_event() 3 times. First and
>> last time with logfunc+logdata and in the middle without. So
>> log_fn_called is 2 afterwards as the one in the middle is omitted.
>
> yes, indeed. the point of this was to check that the logfunc is only called
> when it is set, so  we set it, unset it, set it again and then expect that
> it was called two times (not three times).

hmm... I think I had something in my eyes when reviewing this one. I
miss the call to libevdev_set_log_function(NULL, NULL); which was left
from the previous version.

Thanks for the clarifications both of you :)

Cheers,
Benjamin

>
> I've added a comment to that effect.
>
> There should be another test checking for log data being valid, I'll add
> that separately.
>
> Cheers,
>    Peter