Precis:
This is the final version of Bo Yang's patch, rather than really being a
review, I'm posting this to the list to explain a few things for Bo and for
reference if anyone else works with one of the libs with tests.
Specifically the issue was that Bo hadn't actually tried compiling the tests
and running them. Test suites exist for a reason, which is why I asked for a
test in my first review of Bo's patch. Unfortunately, while tests were added as
per my request, they didn't compile.
It's important to run:
make BUILD=debug test
*AND*
make BUILD=release test
in libraries such as libwapcaplet, because some tests only run in debug mode
because they actually test that the asserts fire. Asserts are, naturally, not
compiled into a release binary.
Also, in something like libwapcaplet, the style is to include every header you
need in your header files directly. i.e. since Bo used uint32_t in the API, he
should have put #include <stdint.h> into it. I didn't catch all this on my
previous reviews because I had made the false assumption that he had compiled
and run the tests. I shall try not to make this mistake in future.
This isn't meant to be a rant, just an indication of some extra work to do
before you request reviews in future. This applies to everyone, not just Bo.
Added files
Changed files
include/libwapcaplet/libwapcaplet.h | 11 +++++++++++
src/libwapcaplet.c | 9 ++++++++-
test/basictests.c | 16 ++++++++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)
Index: test/basictests.c
===================================================================
--- test/basictests.c (revision 6881)
+++ test/basictests.c (working copy)
@@ -174,6 +174,12 @@ START_TEST (test_lwc_string_length_abort
}
END_TEST
+START_TEST (test_lwc_string_hash_value_aborts)
+{
+ lwc_string_hash_value(NULL);
+}
+END_TEST
+
#endif
START_TEST (test_lwc_context_creation_ok)
@@ -381,6 +387,12 @@ START_TEST (test_lwc_extract_data_ok)
}
END_TEST
+START_TEST (test_lwc_string_hash_value_ok)
+{
+ lwc_string_hash_value(intern_one);
+}
+END_TEST
+
/**** And the suites are set up here ****/
void
@@ -444,6 +456,9 @@ lwc_basic_suite(SRunner *sr)
tcase_add_test_raise_signal(tc_basic,
test_lwc_string_length_aborts,
SIGABRT);
+ tcase_add_test_raise_signal(tc_basic,
+ test_lwc_string_hash_value_aborts,
+ SIGABRT);
#endif
tcase_add_test(tc_basic, test_lwc_context_creation_ok);
@@ -471,6 +486,7 @@ lwc_basic_suite(SRunner *sr)
tcase_add_test(tc_basic, test_lwc_context_string_isequal_ok);
tcase_add_test(tc_basic, test_lwc_context_string_caseless_isequal_ok);
tcase_add_test(tc_basic, test_lwc_extract_data_ok);
+ tcase_add_test(tc_basic, test_lwc_string_hash_value_ok);
suite_add_tcase(s, tc_basic);
srunner_add_suite(sr, s);
Index: include/libwapcaplet/libwapcaplet.h
===================================================================
--- include/libwapcaplet/libwapcaplet.h (revision 6881)
+++ include/libwapcaplet/libwapcaplet.h (working copy)
@@ -11,6 +11,7 @@
#include <sys/types.h>
#include <stdbool.h>
+#include <stdint.h>
/**
* Memory allocator type
@@ -126,5 +127,15 @@ extern const char *lwc_string_data(lwc_s
*/
extern size_t lwc_string_length(lwc_string *str);
+/**
+ * Retrieve (or compute if unavailable) a hash value for the content of the
string.
+ *
+ * @note This API should only be used as a convenient way to retrieve a hash
+ * value for the string. This hash value should not be relied on to be
+ * unique within an invocation of the program, nor should it be relied
upon
+ * to be stable between invocations of the program. Never use the hash
+ * value as a way to directly identify the value of the string.
+ */
+extern uint32_t lwc_string_hash_value(lwc_string *str);
#endif /* libwapcaplet_h_ */
Index: src/libwapcaplet.c
===================================================================
--- src/libwapcaplet.c (revision 6881)
+++ src/libwapcaplet.c (working copy)
@@ -7,7 +7,6 @@
*/
#include <string.h>
-#include <stdint.h>
#include <assert.h>
#include "libwapcaplet/libwapcaplet.h"
@@ -342,3 +341,11 @@ lwc_string_length(lwc_string *str)
return str->len;
}
+
+uint32_t
+lwc_string_hash_value(lwc_string *str)
+{
+ assert(str);
+
+ return str->hash;
+}
Conflicted files
Removed files
