Re: [systemd-devel] [PATCH] test-hashmap.c: added unit-test for hashmap

2013-05-02 Thread Daniel Buch
Thanks for review, kay!

I adabted the code. So trivial_cmp() is tested with INT_TO_PTR('a') - i
guess its ok to use 'a' since its evaulated to int?
The casts.. Dont know :p they are removed :)


2013/4/26 Kay Sievers k...@vrfy.org

 On Fri, Apr 26, 2013 at 8:49 PM, Daniel Buch boogiewasth...@gmail.com
 wrote:
  Version 2, with leak fix. This should be good enough to be pushed.

  +static void test_uint64_compare_func(void) {
  +assert_se((uint64_t)trivial_compare_func(a, a) ==
 (uint64_t)0);
  +assert_se((uint64_t)trivial_compare_func(a, b) ==
 (uint64_t)-1);
  +assert_se((uint64_t)trivial_compare_func(b, a) ==
 (uint64_t)1);
  +}
  +
  +static void test_trivial_compare_func(void) {
  +assert_se(trivial_compare_func(a, a) == 0);
  +assert_se(trivial_compare_func(a, b) == -1);
  +assert_se(trivial_compare_func(b, a) == 1);
  +}

 We cannot use trivial here to compare string values and expect
 predictable results. Trivial will just compare the raw pointers,
 which the compiler can arrange as it likes to; a is not necessarily
 the same as another a. No doubt, the hashmap.c code should get some
 comments here explaining things and how they can/should be used, it's
 really not obvious what's going on.

 We can only compare numeric values with INT_TO_PTR(333) or something like
 that.

 What is the  (uint64_t) cast supposed to test here? And we cast a -1
 to an unsigned? That's intentional?

 Thanks,
 Kay

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] test-hashmap.c: added unit-test for hashmap

2013-05-02 Thread Kay Sievers
On Thu, May 2, 2013 at 10:31 AM, Daniel Buch boogiewasth...@gmail.com wrote:
 Thanks for review, kay!

 I adabted the code. So trivial_cmp() is tested with INT_TO_PTR('a') - i
 guess its ok to use 'a' since its evaulated to int?
 The casts.. Dont know :p they are removed :)

Sounds good. You sent the update already?

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] test-hashmap.c: added unit-test for hashmap

2013-05-02 Thread Kay Sievers
On Thu, May 2, 2013 at 11:50 PM, Daniel Buch boogiewasth...@gmail.com wrote:
 Updated version

Applied.

Thanks a lot for doing all that,
Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel