Merge lp:~stefan.goetz-deactivatedaccount/hipl/lib-core-fixes into lp:hipl

Proposed by Stefan Götz
Status: Merged
Merged at revision: 5091
Proposed branch: lp:~stefan.goetz-deactivatedaccount/hipl/lib-core-fixes
Merge into: lp:hipl
Prerequisite: lp:~stefan.goetz-deactivatedaccount/hipl/unittests
Diff against target: 951 lines (+231/-470)
9 files modified
hipd/cert.c (+9/-13)
hipd/hidb.c (+1/-1)
lib/core/conf.c (+18/-26)
lib/core/hit.c (+43/-70)
lib/core/hit.h (+3/-6)
lib/core/straddr.c (+48/-150)
lib/core/straddr.h (+3/-6)
test/lib/core/hit.c (+26/-56)
test/lib/core/straddr.c (+80/-142)
To merge this branch: bzr merge lp:~stefan.goetz-deactivatedaccount/hipl/lib-core-fixes
Reviewer Review Type Date Requested Status
HIPL core team Pending
Review via email: mp+38825@code.launchpad.net

Description of the change

This branch fixes the files lib/core/hit.c and lib/core/straddr.c so that they 1) no longer fail the unit tests in the unittests branch and 2) are generally saner and easier to work, including much improved documentation.

This branch complements the branch lp:~stefan.goetz/hipl/unittests

To post a comment you must log in.
Revision history for this message
Stefan Götz (stefan.goetz-deactivatedaccount) wrote :

Hi Diego!

Thanks for taking the time to review!

> This [the branch] looks like it could be split into two parts, one for the
> fixes and one for the unit tests...

Technically, it could. Logically however, the intention is to start the 'don't commit things that break the unit tests' policy.

>> --- lib/core/hit.c 2010-10-18 17:44:31 +0000
>> +++ lib/core/hit.c 2010-10-19 12:14:42 +0000
>> @@ -42,71 +42,48 @@
>>
>> + if (hit != NULL && hit_str != NULL) {
>> + if (hip_in6_ntop(hit, hit_str) != NULL) {
>
> I don't think the '!= NULL' expressions aid readability.

As opposed to writing "if (hit && hit_str)"? In that case I agree and I would change the code accordingly. Please let me know if it is a different alternative you are suggesting.

> In any case these two if clauses can be combined.

Technically, they could. Logically, there are two different checks here. First, I check my input (i.e. hit and hit_str), then I do the work (hip_in6_ntop()). In my eyes, these two steps are worth separating 1) for comprehensibility and 2) if changes become necessary. I concede that this is a purely philosophical point :)

>> --- test/lib/core/hit.c 2010-10-19 12:14:42 +0000
>> +++ test/lib/core/hit.c 2010-10-19 12:14:42 +0000
>
> This is a separate thing.

True. This is reflected by the fact that on this branch, one commit contains the changes in straddr.c and a different commit those in hit.c.

I hope these explanations adequately address your concerns.

Stefan

5003. By Stefan Götz

In response to https://code.launchpad.net/~stefan.goetz/hipl/lib-core-fixes/+merge/38825/comments/87700:
Removed NULL constants from validity checks as they were felt as being unnecessarily verbose.

5004. By Stefan Götz

Pulled lp:~stefan.goetz/hipl/unittests for mergeability

Revision history for this message
Stefan Götz (stefan.goetz-deactivatedaccount) wrote :

Revision 5003 addresses the concerns about NULL pointer checks raised by Diego. It should thus fix out all outstanding merge blockers. Revision 5004 ensures mergeability on top of lp:~stefan.goetz/hipl/unittests. The branch merges cleanly into the current trunk revision 5028. Unless there are further issues, I herewith re-request this branch to be merged into trunk :-)

5005. By Stefan Götz

Removed unnecessarily included header files from lib/core/straddr.c

5006. By Stefan Götz

Removed unnecessarily included header files from lib/core/hit.c

5007. By Stefan Götz

Pulled lp:~stefan.goetz/hipl/unittests for mergeability

Revision history for this message
Diego Biurrun (diego-biurrun) wrote :

On Tue, Oct 19, 2010 at 04:26:03PM +0000, Stefan Götz wrote:
>
> > This [the branch] looks like it could be split into two parts, one for the
> > fixes and one for the unit tests...
>
> Technically, it could. Logically however, the intention is to start
> the 'don't commit things that break the unit tests' policy.

Let that policy take effect one commit later. I'm sure nobody will be
able to slip a mickey into HIPL between REV_FIXES and REV_FIXES + 1.

> >> --- lib/core/hit.c 2010-10-18 17:44:31 +0000
> >> +++ lib/core/hit.c 2010-10-19 12:14:42 +0000
> >> @@ -42,71 +42,48 @@
> >>
> >> + if (hit != NULL && hit_str != NULL) {
> >> + if (hip_in6_ntop(hit, hit_str) != NULL) {
> >
> > I don't think the '!= NULL' expressions aid readability.
>
> As opposed to writing "if (hit && hit_str)"?

Yes.

> > In any case these two if clauses can be combined.
>
> Technically, they could. Logically, there are two different checks
> here. First, I check my input (i.e. hit and hit_str), then I do
> the work (hip_in6_ntop()). In my eyes, these two steps are worth
> separating 1) for comprehensibility and 2) if changes become
> necessary. I concede that this is a purely philosophical point :)

Whatever you prefer.

Notice that I always consider my review comments suggestions to be
followed or ignored at the coder's discretion. Unless I'm pointing
out bugs of course :)

Diego

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hipd/cert.c'
2--- hipd/cert.c 2010-10-15 15:29:14 +0000
3+++ hipd/cert.c 2010-10-20 08:51:06 +0000
4@@ -194,11 +194,11 @@
5 /* clearing signature field just to be sure */
6 memset(cert->signature, '\0', sizeof(cert->signature));
7
8- HIP_IFEL(!(digest_b64 = base64_encode((unsigned char *) sha_digest,
9- (unsigned int) sizeof(sha_digest))),
10+ HIP_IFEL(EVP_EncodeBlock(digest_b64, (unsigned char *) sha_digest,
11+ (unsigned int) sizeof(sha_digest)) > 0,
12 -1, "Failed to encode digest_b64\n");
13- HIP_IFEL(!(signature_b64 = base64_encode((unsigned char *) signature,
14- (unsigned int) sig_len)),
15+ HIP_IFEL(EVP_EncodeBlock(signature_b64, (unsigned char *) signature,
16+ (unsigned int) sig_len) > 0,
17 -1, "Failed to encode signature_b64\n");
18 /* create (signature (hash sha1 |digest|)|signature|) */
19 sprintf(cert->signature, "(signature (hash sha1 |%s|)|%s|)",
20@@ -218,7 +218,7 @@
21 -1,
22 "Error in converting public exponent from BN to bin\n");
23
24- HIP_IFEL(!(n_b64 = base64_encode((unsigned char *) n_bin, RSA_size(rsa))),
25+ HIP_IFEL(EVP_EncodeBlock(n_b64, (unsigned char *) n_bin, RSA_size(rsa)) > 0,
26 -1,
27 "Failed to encode n_b64\n");
28
29@@ -244,26 +244,22 @@
30 */
31 HIP_IFEL(!(BN_bn2bin(dsa->p, p_bin)), -1,
32 "Error in converting public exponent from BN to bin\n");
33- HIP_IFEL(!(p_b64 = base64_encode((unsigned char *) p_bin,
34- BN_num_bytes(dsa->p))),
35+ HIP_IFEL(EVP_EncodeBlock(p_b64, (unsigned char *) p_bin, BN_num_bytes(dsa->p)) > 0,
36 -1, "Failed to encode p_b64\n");
37
38 HIP_IFEL(!(BN_bn2bin(dsa->q, q_bin)), -1,
39 "Error in converting public exponent from BN to bin\n");
40- HIP_IFEL(!(q_b64 = base64_encode((unsigned char *) q_bin,
41- BN_num_bytes(dsa->q))),
42+ HIP_IFEL(EVP_EncodeBlock(q_b64, (unsigned char *) q_bin, BN_num_bytes(dsa->q)) > 0,
43 -1, "Failed to encode q_64");
44
45 HIP_IFEL(!(BN_bn2bin(dsa->g, g_bin)), -1,
46 "Error in converting public exponent from BN to bin\n");
47- HIP_IFEL(!(g_b64 = base64_encode((unsigned char *) g_bin,
48- BN_num_bytes(dsa->g))),
49+ HIP_IFEL(EVP_EncodeBlock(g_b64, (unsigned char *) g_bin, BN_num_bytes(dsa->g)) > 0,
50 -1, "Failed to encode g_b64\n");
51
52 HIP_IFEL(!(BN_bn2bin(dsa->pub_key, y_bin)), -1,
53 "Error in converting public exponent from BN to bin\n");
54- HIP_IFEL(!(y_b64 = base64_encode((unsigned char *) y_bin,
55- BN_num_bytes(dsa->pub_key))),
56+ HIP_IFEL(EVP_EncodeBlock(y_b64, (unsigned char *) y_bin, BN_num_bytes(dsa->pub_key)) > 0,
57 -1, "Failed to encode y_b64\n");
58
59 sprintf(cert->public_key, "(public_key (dsa-pkcs1-sha1 (p |%s|)(q |%s|)"
60
61=== modified file 'hipd/hidb.c'
62--- hipd/hidb.c 2010-10-15 15:29:14 +0000
63+++ hipd/hidb.c 2010-10-20 08:51:06 +0000
64@@ -355,7 +355,7 @@
65
66 list_for_each(item, hip_local_hostid_db, c) {
67 id_entry = (struct hip_host_id_entry *) list_entry(item);
68- if (hip_hit_are_equal(&id_entry->lhi.hit, our)) {
69+ if (memcmp(&id_entry->lhi.hit, our, sizeof(*our)) == 0) {
70 memcpy(our_lsi, &id_entry->lsi, sizeof(hip_lsi_t));
71 return 0;
72 }
73
74=== modified file 'lib/core/conf.c'
75--- lib/core/conf.c 2010-10-20 03:38:26 +0000
76+++ lib/core/conf.c 2010-10-20 08:51:06 +0000
77@@ -872,7 +872,6 @@
78 int index_of_hit = 0, index_of_ip = 0, opp_mode = 0;;
79 uint8_t lifetime = 0, *reg_types = NULL;
80 time_t seconds_from_lifetime = 0;
81- char lowercase[30];
82
83 memset(&hit, 0, sizeof(hit));
84 memset(&ipv6, 0, sizeof(ipv6));
85@@ -883,6 +882,8 @@
86 err = -1;
87 goto out_err;
88 } else if (action == ACTION_ADD) {
89+ char *tail_ptr = NULL;
90+
91 if (optc < 4) {
92 if (optc < 3) {
93 HIP_ERROR("Missing arguments.\n");
94@@ -903,20 +904,12 @@
95 index_of_ip = optc - 2;
96 }
97
98- HIP_IFEL(hip_string_is_digit(opt[optc - 1]), -1,
99+ seconds = strtoul(opt[optc - 1], &tail_ptr, 10);
100+ HIP_IFEL(*tail_ptr == '\0' && seconds > 0 && seconds <= 15384774, -1,
101 "Invalid lifetime value \"%s\" given.\n" \
102 "Please give a lifetime value between 1 and " \
103 "15384774 seconds.\n", opt[optc - 1]);
104
105- seconds = atoi(opt[optc - 1]);
106-
107- if (seconds <= 0 || seconds > 15384774) {
108- HIP_ERROR("Invalid lifetime value \"%s\" given.\n" \
109- "Please give a lifetime value between 1 and " \
110- "15384774 seconds.\n", opt[optc - 1]);
111- goto out_err;
112- }
113-
114 HIP_IFEL(hip_get_lifetime_value(seconds, &lifetime), -1,
115 "Unable to convert seconds to a lifetime value.\n");
116
117@@ -994,19 +987,18 @@
118 goto out_err;
119 }
120
121- hip_string_to_lowercase(lowercase, opt[i], strlen(opt[i]) + 1);
122- if (strcmp("rvs", lowercase) == 0) {
123+ if (strcasecmp("rvs", opt[i]) == 0) {
124 reg_types[i] = HIP_SERVICE_RENDEZVOUS;
125- } else if (strcmp("relay", lowercase) == 0) {
126+ } else if (strcasecmp("relay", opt[i]) == 0) {
127 reg_types[i] = HIP_SERVICE_RELAY;
128- } else if (strcmp("full-relay", lowercase) == 0) {
129+ } else if (strcasecmp("full-relay", opt[i]) == 0) {
130 reg_types[i] = HIP_SERVICE_FULLRELAY;
131 } /* To cope with the atoi() error value we handle the 'zero'
132 * case here. */
133- else if (strcmp("0", lowercase) == 0) {
134+ else if (strcasecmp("0", opt[i]) == 0) {
135 reg_types[i] = 0;
136 } else {
137- reg_type = atoi(lowercase);
138+ reg_type = atoi(opt[i]);
139 if (reg_type <= 0 || reg_type > 255) {
140 HIP_ERROR("'%s' is not a valid service name " \
141 "or service number.\n", opt[i]);
142@@ -1207,13 +1199,13 @@
143
144 HIP_IFEL((optc != 2 && optc != 3), -1, "Missing arguments\n");
145
146- HIP_IFEL(convert_string_to_address(opt[0], &hit), -1,
147- "string to address conversion failed\n");
148-
149- HIP_IFEL((err = convert_string_to_address(opt[1], &ip6)), -1,
150- "string to address conversion failed\n");
151-
152- if ((err && !convert_string_to_address_v4(opt[1], &aux))) {
153+ HIP_IFEL(hip_convert_string_to_address(opt[0], &hit), -1,
154+ "string to address conversion failed\n");
155+
156+ HIP_IFEL((err = hip_convert_string_to_address(opt[1], &ip6)), -1,
157+ "string to address conversion failed\n");
158+
159+ if ((err && inet_pton(AF_INET, opt[1], &aux) != 1)) {
160 HIP_IFEL(IS_LSI32(aux.s_addr), -1, "Missing ip address before lsi\n");
161 }
162
163@@ -1243,7 +1235,7 @@
164 "build param hit failed\n");
165
166 if (optc == 3) {
167- HIP_IFEL(convert_string_to_address_v4(opt[2], &lsi), -1,
168+ HIP_IFEL(inet_pton(AF_INET, opt[2], &lsi) == 1, -1,
169 "string to address conversion failed\n");
170 HIP_IFEL(!IS_LSI32(lsi.s_addr), -1, "Wrong LSI value\n");
171 HIP_IFEL(hip_build_param_contents(msg, &lsi,
172@@ -2011,7 +2003,7 @@
173 if (!strcmp("all", opt[0])) {
174 hip_conf_print_info_ha(ha);
175 } else {
176- HIP_IFE(convert_string_to_address(opt[0], &hit1), -1);
177+ HIP_IFE(hip_convert_string_to_address(opt[0], &hit1), -1);
178
179 if ((ipv6_addr_cmp(&hit1, &ha->hit_our) == 0) ||
180 (ipv6_addr_cmp(&hit1, &ha->hit_peer) == 0))
181
182=== modified file 'lib/core/hit.c'
183--- lib/core/hit.c 2010-10-18 17:44:31 +0000
184+++ lib/core/hit.c 2010-10-20 08:51:06 +0000
185@@ -30,83 +30,56 @@
186 * @author Miika Komu <miika@iki.fi>
187 */
188
189-#include <stdint.h>
190-#include <string.h>
191+#include <string.h> // strcpy()
192
193-#include "config.h"
194-#include "builder.h"
195-#include "debug.h"
196-#include "prefix.h"
197-#include "protodefs.h"
198-#include "straddr.h"
199+#include "debug.h" // HIP_ASSERT()
200+#include "prefix.h" // ipv6_addr_cmp()
201+#include "straddr.h" // hip_in6_ntop()
202 #include "hit.h"
203
204 /**
205- * convert a binary HIT into a string
206+ * Convert a binary HIT to a hexadecimal string representation of the form
207+ * 0011:2233:4455:6677:8899:AABB:CCDD:EEFF terminated by a null character.
208 *
209- * @param hit a binary HIT
210- * @param prefix an optional HIT prefix as a string
211- * @param hit_str the HIT as a string with the given prefix
212- * @return zero on success and negative on error
213+ * @param hit a pointer to a binary HIT.
214+ * @param suffix an optional null-terminated string suffix to be appended to
215+ * the HIT. If suffix is NULL or the empty string, no suffix is appended. If
216+ * suffix is not null-terminated, the result is undefined.
217+ * @param hit_str a pointer to a buffer to write the HIT and the suffix to. The
218+ * result of passing a buffer that is too short to hold the string
219+ * representation plus the suffix is undefined.
220+ * @return 0 if the HIT was successfully converted. Returns a negative value if
221+ * hit is NULL or hit_str is NULL.
222 */
223-int hip_convert_hit_to_str(const hip_hit_t *hit,
224- const char *prefix,
225- char *hit_str)
226+int hip_convert_hit_to_str(const hip_hit_t *const hit,
227+ const char *const suffix,
228+ char *const hit_str)
229 {
230- int err = 0;
231-
232- HIP_ASSERT(hit);
233-
234- memset(hit_str, 0, INET6_ADDRSTRLEN);
235- err = !hip_in6_ntop(hit, hit_str);
236-
237- if (prefix) {
238- memcpy(hit_str + strlen(hit_str), prefix, strlen(prefix));
239+ if (hit && hit_str) {
240+ if (hip_in6_ntop(hit, hit_str)) {
241+ if (suffix && *suffix != '\0') {
242+ strcpy(hit_str + strlen(hit_str), suffix);
243+ }
244+ return 0;
245+ }
246 }
247
248- return err;
249-}
250-/**
251- * compare two HITs to check which HIT is "bigger"
252- *
253- * @param hit1 the first HIT to be compared
254- * @param hit2 the second HIT to be compared
255- *
256- * @return 1 if hit1 was bigger than hit2, or else 0
257- */
258-int hip_hit_is_bigger(const struct in6_addr *hit1,
259- const struct in6_addr *hit2)
260-{
261- return ipv6_addr_cmp(hit1, hit2) > 0;
262-}
263-
264-/**
265- * compare two HITs to check which if they are equal
266- *
267- * @param hit1 the first HIT to be compared
268- * @param hit2 the second HIT to be compared
269- *
270- * @return 1 if the HITs were equal and zero otherwise
271- */
272-int hip_hit_are_equal(const struct in6_addr *hit1,
273- const struct in6_addr *hit2)
274-{
275- return ipv6_addr_cmp(hit1, hit2) == 0;
276-}
277-
278-/**
279- * calculate a hash from a HIT
280- *
281- * @param ptr pointer to a HIT
282- *
283- * Returns value in range: 0 <= x < range
284- */
285-unsigned long hip_hash_hit(const void *ptr)
286-{
287- uint8_t hash[HIP_AH_SHA_LEN];
288-
289- hip_build_digest(HIP_DIGEST_SHA1, (const uint8_t *)ptr + sizeof(uint16_t),
290- 7 * sizeof(uint16_t), hash);
291-
292- return *((unsigned long *) hash);
293+ return -1;
294+}
295+
296+/**
297+ * Determine whether a HIT is numerically greater than another.
298+ *
299+ * @param hit_gt a pointer to a HIT. When passing a NULL pointer, the result
300+ * of this function is undefined.
301+ * @param hit_le a pointer to a HIT. When passing a NULL pointer, the result
302+ * of this function is undefined.
303+ * @return 1 if hit_gt is greater than hit_le, otherwise 0.
304+ */
305+int hip_hit_is_bigger(const struct in6_addr *const hit_gt,
306+ const struct in6_addr *const hit_le)
307+{
308+ HIP_ASSERT(hit_gt);
309+ HIP_ASSERT(hit_le);
310+ return ipv6_addr_cmp(hit_gt, hit_le) > 0;
311 }
312
313=== modified file 'lib/core/hit.h'
314--- lib/core/hit.h 2010-10-15 15:29:14 +0000
315+++ lib/core/hit.h 2010-10-20 08:51:06 +0000
316@@ -30,11 +30,8 @@
317
318 #include "protodefs.h"
319
320-int hip_convert_hit_to_str(const hip_hit_t *hit, const char *prefix, char *str);
321-int hip_hit_is_bigger(const struct in6_addr *hit1,
322- const struct in6_addr *hit2);
323-int hip_hit_are_equal(const struct in6_addr *hit1,
324- const struct in6_addr *hit2);
325-unsigned long hip_hash_hit(const void *hit);
326+int hip_convert_hit_to_str(const hip_hit_t *const hit, const char *const suffix, char *const str);
327+int hip_hit_is_bigger(const struct in6_addr *const hit_gt,
328+ const struct in6_addr *const hit_le);
329
330 #endif /* HIP_LIB_CORE_HIT_H */
331
332=== modified file 'lib/core/straddr.c'
333--- lib/core/straddr.c 2010-10-18 17:44:31 +0000
334+++ lib/core/straddr.c 2010-10-20 08:51:06 +0000
335@@ -32,30 +32,28 @@
336
337 #define _BSD_SOURCE
338
339-#include <ctype.h>
340-#include <errno.h>
341-#include <stdio.h>
342-#include <stdlib.h>
343-#include <arpa/inet.h>
344-#include <netinet/in.h>
345-#include <openssl/evp.h>
346+#include <stdio.h> // sprintf()
347+#include <arpa/inet.h> // inet_pton()
348
349-#include "config.h"
350-#include "debug.h"
351-#include "ife.h"
352-#include "prefix.h"
353+#include "debug.h" // HIP_DEBUG()
354+#include "prefix.h" // IPV4_TO_IPV6_MAP()
355 #include "straddr.h"
356
357 /**
358- * convert a binary IPv6 address to a string
359+ * Convert a binary IPv6 address to a hexadecimal string representation of the
360+ * form 0011:2233:4455:6677:8899:AABB:CCDD:EEFF terminated by a null character.
361 *
362- * @param in6 the IPv6 address to convert
363- * @param buf a preallocated buffer where the string will be stored
364- * @return a pointer to the buf
365+ * @param in6 a pointer to a binary IPv6 address.
366+ * @param buf a pointer to a buffer to write the string representation to. The
367+ * result of passing a buffer that is too short to hold the string
368+ * representation is undefined.
369+ * @return The function returns a pointer to the output buffer buf if the
370+ * address is successfully converted. It returns a negative value if in6 is
371+ * NULL or buf is NULL.
372 */
373-char *hip_in6_ntop(const struct in6_addr *in6, char *buf)
374+char *hip_in6_ntop(const struct in6_addr *const in6, char *const buf)
375 {
376- if (!buf) {
377+ if (!in6 || !buf) {
378 return NULL;
379 }
380 sprintf(buf,
381@@ -68,137 +66,37 @@
382 }
383
384 /**
385- * convert a string into a binary IPv4 address (a wrapper for inet_pton())
386- *
387- * @param str the string to convert
388- * @param ip an output argument that will contain a binary IPv4 calculated
389- * from the @c str
390- * @return zero on success and negative on error
391- */
392-int convert_string_to_address_v4(const char *str, struct in_addr *ip)
393-{
394- int ret = 0, err = 0;
395-
396- ret = inet_pton(AF_INET, str, ip);
397- HIP_IFEL((ret < 0 && errno == EAFNOSUPPORT), -1,
398- "inet_pton: not a valid address family\n");
399- HIP_IFEL((ret == 0), -1,
400- "inet_pton: %s: not a valid network address\n", str);
401-out_err:
402- return err;
403-}
404-
405-/**
406- * Convert a string to an IPv6 address. This function can handle
407- * also IPv6 mapped addresses.
408- *
409- * @param str the string to convert
410- * @param ip6 An output argument that will contain a binary IPv4 calculated
411- * from the @c str. Possibly in IPv6 mapped format.
412- * @return zero on success or negative on error
413- */
414-int convert_string_to_address(const char *str,
415- struct in6_addr *ip6)
416-{
417- int ret = 0, err = 0;
418- struct in_addr ip4;
419-
420- ret = inet_pton(AF_INET6, str, ip6);
421- HIP_IFEL((ret < 0 && errno == EAFNOSUPPORT), -1,
422- "\"%s\" is not of valid address family.\n", str);
423- if (ret > 0) {
424- /* IPv6 address conversion was ok */
425- goto out_err;
426- }
427-
428- /* Might be an ipv4 address (ret == 0). Lets catch it here. */
429- err = convert_string_to_address_v4(str, &ip4);
430- if (err) {
431- goto out_err;
432- }
433-
434- IPV4_TO_IPV6_MAP(&ip4, ip6);
435- HIP_DEBUG("Mapped v4 to v6.\n");
436- HIP_DEBUG_IN6ADDR("mapped v6", ip6);
437-
438-out_err:
439- return err;
440-}
441-
442-/**
443- * convert a string containing upper case characters to lower case
444- *
445- * @param to the result of the conversion (minimum length @c count)
446- * @param from a string possibly containing upper case characters
447- * @param count count
448- * @return zero on success or negative on failure
449- */
450-int hip_string_to_lowercase(char *to, const char *from, const size_t count)
451-{
452- unsigned i;
453-
454- if (to == NULL || from == NULL || count == 0) {
455- return -1;
456- }
457-
458- for (i = 0; i < count; i++) {
459- if (isalpha(from[i])) {
460- to[i] = tolower(from[i]);
461+ * Convert a string representation of an IPv6 or IPv4 address to a struct
462+ * in6_addr.
463+ * If the string contains an IPv4 address, it is converted to its
464+ * IPv6-compatible mapping.
465+ *
466+ * @param str points to the string to convert.
467+ * @param ip6 points to a buffer where the function stores the binary address
468+ * if it could be converted.
469+ * @return The return value is 0 if the conversion succeeds. It is a
470+ * negative value if str or ip6 are NULL or if str contains neither a
471+ * parseable IPv6 or IPv4 address.
472+ */
473+int hip_convert_string_to_address(const char *const str,
474+ struct in6_addr *const ip6)
475+{
476+ if (str && ip6) {
477+ if (inet_pton(AF_INET6, str, ip6) == 1) {
478+ /* IPv6 address conversion was ok */
479+ return 0;
480 } else {
481- to[i] = from[i];
482- }
483- }
484- return 0;
485-}
486-
487-/**
488- * test if a given string contains a positive integer
489- *
490- * @param string the string to test
491- * @return zero if the string is digit or negative otherwise
492- */
493-int hip_string_is_digit(const char *string)
494-{
495- if (string == NULL) {
496- return -1;
497- }
498-
499- int i = 0;
500-
501- while (string[i] != '\0') {
502- if (!isdigit(string[i])) {
503- return -1;
504- }
505- i++;
506- }
507- return 0;
508-}
509-
510-
511-/**
512- * encode the given content to Base64
513- *
514- * @param buf Pointer to contents to be encoded
515- * @param len How long is the first parameter in bytes
516- *
517- * @return Returns a pointer to encoded content or NULL on error
518- */
519-unsigned char *base64_encode(unsigned char *buf, unsigned int len)
520-{
521- unsigned char *ret;
522- unsigned int b64_len;
523-
524- b64_len = (((len + 2) / 3) * 4) + 1;
525- ret = malloc(b64_len);
526- if (ret == NULL) {
527- goto out_err;
528- }
529- EVP_EncodeBlock(ret, buf, len);
530- return ret;
531-out_err:
532- if (ret) {
533- free(ret);
534- }
535- return NULL;
536-}
537-
538+ struct in_addr ip4;
539+
540+ /* Might be an ipv4 address (ret == 0). Lets catch it here. */
541+ if (inet_pton(AF_INET, str, &ip4) == 1) {
542+ IPV4_TO_IPV6_MAP(&ip4, ip6);
543+ HIP_DEBUG("Mapped v4 to v6.\n");
544+ HIP_DEBUG_IN6ADDR("mapped v6", ip6);
545+ return 0;
546+ }
547+ }
548+ }
549+
550+ return -1;
551+}
552
553=== modified file 'lib/core/straddr.h'
554--- lib/core/straddr.h 2010-10-15 15:29:14 +0000
555+++ lib/core/straddr.h 2010-10-20 08:51:06 +0000
556@@ -29,11 +29,8 @@
557 #include <sys/types.h>
558 #include <netinet/in.h>
559
560-int convert_string_to_address_v4(const char *str, struct in_addr *ip);
561-int convert_string_to_address(const char *str, struct in6_addr *ip6);
562-char *hip_in6_ntop(const struct in6_addr *in6, char *buf);
563-int hip_string_to_lowercase(char *to, const char *from, const size_t count);
564-int hip_string_is_digit(const char *string);
565-unsigned char *base64_encode(unsigned char *, unsigned int);
566+char *hip_in6_ntop(const struct in6_addr *const in6, char *const buf);
567+int hip_convert_string_to_address(const char *const str,
568+ struct in6_addr *const ip6);
569
570 #endif /* HIP_LIB_CORE_STRADDR_H */
571
572=== modified file 'test/lib/core/hit.c'
573--- test/lib/core/hit.c 2010-10-20 08:51:06 +0000
574+++ test/lib/core/hit.c 2010-10-20 08:51:06 +0000
575@@ -44,18 +44,18 @@
576 START_TEST(test_hip_convert_hit_to_str_null_hit)
577 {
578 char buf[64];
579- hip_convert_hit_to_str(NULL, "", buf);
580+ fail_unless(hip_convert_hit_to_str(NULL, "", buf) < 0, NULL);
581 }
582 END_TEST
583
584 START_TEST(test_hip_convert_hit_to_str_null_buf)
585 {
586 hip_hit_t hit;
587- fail_unless(hip_convert_hit_to_str(&hit, "", NULL) == 1, NULL);
588+ fail_unless(hip_convert_hit_to_str(&hit, "", NULL) < 0, NULL);
589 }
590 END_TEST
591
592-START_TEST(test_hip_convert_hit_to_str_null_prefix)
593+START_TEST(test_hip_convert_hit_to_str_null_suffix)
594 {
595 char buf[64];
596 hip_hit_t hit;
597@@ -70,9 +70,12 @@
598 const unsigned int HIT_LEN = 39; // 16 bytes -> 32 hex chars + 7 ':'s
599 const unsigned int SUFFIX_LEN = sizeof(suffix); // includes null char
600 const unsigned int AFTER_LEN = 30;
601- char buf[BEFORE_LEN + HIT_LEN + SUFFIX_LEN + AFTER_LEN] = { 1 };
602- char ones[BEFORE_LEN + HIT_LEN + SUFFIX_LEN + AFTER_LEN] = { 1 };
603+ char buf[BEFORE_LEN + HIT_LEN + SUFFIX_LEN + AFTER_LEN];
604+ char ones[BEFORE_LEN + HIT_LEN + SUFFIX_LEN + AFTER_LEN];
605 hip_hit_t hit;
606+
607+ memset(buf, '1', sizeof(buf));
608+ memset(ones, '1', sizeof(ones));
609 memset(&hit.s6_addr, 0x22, sizeof(hit.s6_addr));
610
611 // write the HIT string into the middle of the buffer
612@@ -107,6 +110,20 @@
613 }
614 END_TEST
615
616+START_TEST(test_hip_hit_is_bigger_null_first)
617+{
618+ const hip_hit_t hit = IN6ADDR_LOOPBACK_INIT;
619+ hip_hit_is_bigger(NULL, &hit);
620+}
621+END_TEST
622+
623+START_TEST(test_hip_hit_is_bigger_null_second)
624+{
625+ const hip_hit_t hit = IN6ADDR_LOOPBACK_INIT;
626+ hip_hit_is_bigger(&hit, NULL);
627+}
628+END_TEST
629+
630 START_TEST(test_hip_hit_is_bigger_first_null)
631 {
632 hip_hit_t hit;
633@@ -121,49 +138,6 @@
634 }
635 END_TEST
636
637-START_TEST(test_hip_hit_are_equal_equality)
638-{
639- const hip_hit_t hit1 = IN6ADDR_LOOPBACK_INIT;
640- const hip_hit_t hit2 = IN6ADDR_LOOPBACK_INIT;
641- fail_unless(hip_hit_are_equal(&hit1, &hit2) == 1, NULL);
642-}
643-END_TEST
644-
645-START_TEST(test_hip_hit_are_equal_inequality)
646-{
647- const hip_hit_t bigger = IN6ADDR_LOOPBACK_INIT;
648- const hip_hit_t smaller = IN6ADDR_ANY_INIT;
649- fail_unless(hip_hit_are_equal(&bigger, &smaller) == 1, NULL);
650-}
651-END_TEST
652-
653-START_TEST(test_hip_hit_are_equal_first_null)
654-{
655- hip_hit_t hit;
656- hip_hit_are_equal(NULL, &hit);
657-}
658-END_TEST
659-
660-START_TEST(test_hip_hit_are_equal_second_null)
661-{
662- hip_hit_t hit;
663- hip_hit_are_equal(&hit, NULL);
664-}
665-END_TEST
666-
667-START_TEST(test_hip_hash_hit_valid)
668-{
669- const hip_hit_t hit = IN6ADDR_ANY_INIT;
670- hip_hash_hit(&hit);
671-}
672-END_TEST
673-
674-START_TEST(test_hip_hash_hit_null)
675-{
676- hip_hash_hit(NULL);
677-}
678-END_TEST
679-
680 // For unknown reasons, this file does not compile with the following,
681 // seemingly useless forward declaration
682 Suite *lib_core_hit(void);
683@@ -174,20 +148,16 @@
684
685 TCase *tc_core = tcase_create("Core");
686 tcase_add_test(tc_core, test_hip_convert_hit_to_str_valid);
687- tcase_add_exit_test(tc_core, test_hip_convert_hit_to_str_null_hit, 1);
688+ tcase_add_test(tc_core, test_hip_convert_hit_to_str_null_hit);
689 tcase_add_test(tc_core, test_hip_convert_hit_to_str_null_buf);
690- tcase_add_test(tc_core, test_hip_convert_hit_to_str_null_prefix);
691+ tcase_add_test(tc_core, test_hip_convert_hit_to_str_null_suffix);
692 tcase_add_test(tc_core, test_hip_convert_hit_to_str_bounds);
693 tcase_add_test(tc_core, test_hip_hit_is_bigger_bigger);
694 tcase_add_test(tc_core, test_hip_hit_is_bigger_equal_smaller);
695+ tcase_add_exit_test(tc_core, test_hip_hit_is_bigger_null_first, 1);
696+ tcase_add_exit_test(tc_core, test_hip_hit_is_bigger_null_second, 1);
697 tcase_add_exit_test(tc_core, test_hip_hit_is_bigger_first_null, 1);
698 tcase_add_exit_test(tc_core, test_hip_hit_is_bigger_second_null, 1);
699- tcase_add_test(tc_core, test_hip_hit_are_equal_equality);
700- tcase_add_test(tc_core, test_hip_hit_are_equal_inequality);
701- tcase_add_exit_test(tc_core, test_hip_hit_are_equal_first_null, 1);
702- tcase_add_exit_test(tc_core, test_hip_hit_are_equal_second_null, 1);
703- tcase_add_test(tc_core, test_hip_hash_hit_valid);
704- tcase_add_exit_test(tc_core, test_hip_hash_hit_null, 1);
705 suite_add_tcase(s, tc_core);
706
707 return s;
708
709=== modified file 'test/lib/core/straddr.c'
710--- test/lib/core/straddr.c 2010-10-20 08:51:06 +0000
711+++ test/lib/core/straddr.c 2010-10-20 08:51:06 +0000
712@@ -31,137 +31,83 @@
713 #include <stdlib.h> // free()
714 #include "lib/core/straddr.h"
715
716-START_TEST(test_convert_string_to_address_v4_valid)
717-{
718- const char *str = "127.0.0.1";
719- struct in_addr ip;
720-
721- fail_unless(convert_string_to_address_v4(str, &ip) == 0, NULL);
722-}
723-END_TEST
724-
725-START_TEST(test_convert_string_to_address_v4_null_str)
726-{
727- struct in_addr ip;
728-
729- fail_unless(convert_string_to_address_v4(NULL, &ip) < 0, NULL);
730-}
731-END_TEST
732-
733-START_TEST(test_convert_string_to_address_v4_null_addr)
734-{
735- const char *str = "127.0.0.1";
736-
737- fail_unless(convert_string_to_address_v4(str, NULL) < 0, NULL);
738-}
739-END_TEST
740-
741-START_TEST(test_convert_string_to_address_v4_invalid)
742-{
743- const char *str = " 127.0.0.1";
744- struct in_addr ip;
745-
746- fail_unless(convert_string_to_address_v4(str, &ip) < 0, NULL);
747-}
748-END_TEST
749-
750-START_TEST(test_convert_string_to_address_valid)
751-{
752- const char *str = "fe80::215:58ff:fe29:9c36";
753- struct in6_addr ip;
754-
755- fail_unless(convert_string_to_address(str, &ip) == 0, NULL);
756-}
757-END_TEST
758-
759-START_TEST(test_convert_string_to_address_null_str)
760-{
761- struct in6_addr ip;
762-
763- fail_unless(convert_string_to_address(NULL, &ip) < 0, NULL);
764-}
765-END_TEST
766-
767-START_TEST(test_convert_string_to_address_null_addr)
768-{
769- const char *str = "fe80::215:58ff:fe29:9c36";
770-
771- fail_unless(convert_string_to_address(str, NULL) < 0, NULL);
772-}
773-END_TEST
774-
775-START_TEST(test_convert_string_to_address_invalid)
776+START_TEST(test_hip_in6_ntop_valid)
777+{
778+ const int GUARD_SIZE = 32; // arbitrary
779+ struct buf_test {
780+ char before[GUARD_SIZE];
781+ char addr[39]; // 16 IPv6 bytes -> 32 hex chars + 7 ':'s
782+ char null[1]; // terminating null character
783+ char after[GUARD_SIZE];
784+ } buf;
785+ char ones[GUARD_SIZE];
786+ struct in6_addr in6;
787+
788+ memset(&buf, '1', sizeof(buf));
789+ memset(ones, '1', sizeof(ones));
790+ memset(&in6.s6_addr, 0x22, sizeof(in6.s6_addr));
791+
792+ fail_unless(hip_in6_ntop(&in6, buf.addr) == buf.addr, NULL);
793+ // is the buffer before the address untouched?
794+ fail_unless(memcmp(buf.before, ones, GUARD_SIZE) == 0, NULL);
795+ // is the first part of the address correct?
796+ fail_unless(buf.addr[0] == '2', NULL);
797+ // is the last part of the address correct?
798+ fail_unless(buf.addr[sizeof(buf.addr) - 1] == '2', NULL);
799+ // is there a terminating null character?
800+ fail_unless(buf.null[0] == '\0', NULL);
801+ // is the buffer after the address untouched?
802+ fail_unless(memcmp(buf.after, ones, GUARD_SIZE) == 0, NULL);
803+}
804+END_TEST
805+
806+START_TEST(test_hip_in6_ntop_null_addr)
807+{
808+ char buf[64];
809+
810+ fail_unless(hip_in6_ntop(NULL, buf) == NULL, NULL);
811+}
812+END_TEST
813+
814+START_TEST(test_hip_in6_ntop_null_buf)
815+{
816+ struct in6_addr in6 = IN6ADDR_LOOPBACK_INIT;
817+
818+ fail_unless(hip_in6_ntop(&in6, NULL) == NULL, NULL);
819+}
820+END_TEST
821+
822+START_TEST(test_hip_convert_string_to_address_valid)
823+{
824+ const char *str = "fe80::215:58ff:fe29:9c36";
825+ struct in6_addr ip;
826+
827+ fail_unless(hip_convert_string_to_address(str, &ip) == 0, NULL);
828+}
829+END_TEST
830+
831+START_TEST(test_hip_convert_string_to_address_null_str)
832+{
833+ struct in6_addr ip;
834+
835+ fail_unless(hip_convert_string_to_address(NULL, &ip) < 0, NULL);
836+}
837+END_TEST
838+
839+START_TEST(test_hip_convert_string_to_address_null_addr)
840+{
841+ const char *str = "fe80::215:58ff:fe29:9c36";
842+
843+ fail_unless(hip_convert_string_to_address(str, NULL) < 0, NULL);
844+}
845+END_TEST
846+
847+START_TEST(test_hip_convert_string_to_address_invalid)
848 {
849 const char *str = " fe80::215:58ff:fe29:9c36";
850 struct in6_addr ip;
851
852- fail_unless(convert_string_to_address(str, &ip) < 0, NULL);
853-}
854-END_TEST
855-
856-START_TEST(test_hip_string_to_lowercase_valid)
857-{
858- char to[128] = { 1 };
859- char ones[128] = { 1 };
860- const char from[] = "TesT";
861- const size_t count = sizeof(from) - 1;
862- const unsigned int offset = 32;
863-
864- fail_unless(hip_string_to_lowercase(to + offset, from, count) == 0, NULL);
865- // was from correctly converted to lower case?
866- fail_unless(memcmp(to + offset, "test", count) == 0, NULL);
867- // is the beginning of to still intact?
868- fail_unless(memcmp(to, ones, offset) == 0, NULL);
869- // is the rest of to still intact?
870- fail_unless(memcmp(to + offset + count, ones, offset) == 0, NULL);
871-}
872-END_TEST
873-
874-START_TEST(test_hip_string_is_digit_valid)
875-{
876- fail_unless(hip_string_is_digit("123456789") == 0, NULL);
877- fail_unless(hip_string_is_digit("abc") < 0, NULL);
878-}
879-END_TEST
880-
881-START_TEST(test_hip_string_is_digit_null)
882-{
883- fail_unless(hip_string_is_digit(NULL) < 0, NULL);
884-}
885-END_TEST
886-
887-START_TEST(test_hip_string_is_digit_empty)
888-{
889- fail_unless(hip_string_is_digit("") < 0, NULL);
890-}
891-END_TEST
892-
893-START_TEST(test_base64_encode_valid)
894-{
895- const char b64[] = "VGVzdA==";
896- unsigned char buf[] = "Test";
897- unsigned int len = sizeof(buf) - 1; // do not include null character as per doc
898- unsigned char *result = NULL;
899-
900- fail_unless((result = base64_encode(buf, len)) != NULL, NULL);
901- fail_unless(strcmp((char*)result, b64) == 0, NULL);
902- free(result); // note it's not documented that we need to free the returned memory
903-}
904-END_TEST
905-
906-START_TEST(test_base64_encode_null_buf)
907-{
908- fail_unless(base64_encode(NULL, 42) == NULL, NULL);
909-}
910-END_TEST
911-
912-START_TEST(test_base64_encode_empty_buf)
913-{
914- unsigned char buf[] = "";
915- unsigned char *result = NULL;
916-
917- fail_unless((result = base64_encode(buf, 0)) != NULL, NULL);
918- fail_unless(strlen((char *)result) == 0, NULL);
919+ fail_unless(hip_convert_string_to_address(str, &ip) < 0, NULL);
920 }
921 END_TEST
922
923@@ -174,21 +120,13 @@
924 Suite *s = suite_create("lib/core/straddr");
925
926 TCase *tc_core = tcase_create("Core");
927- tcase_add_test(tc_core, test_convert_string_to_address_v4_valid);
928- tcase_add_test(tc_core, test_convert_string_to_address_v4_null_str);
929- tcase_add_test(tc_core, test_convert_string_to_address_v4_null_addr);
930- tcase_add_test(tc_core, test_convert_string_to_address_v4_invalid);
931- tcase_add_test(tc_core, test_convert_string_to_address_valid);
932- tcase_add_test(tc_core, test_convert_string_to_address_null_str);
933- tcase_add_test(tc_core, test_convert_string_to_address_null_addr);
934- tcase_add_test(tc_core, test_convert_string_to_address_invalid);
935- tcase_add_test(tc_core, test_hip_string_to_lowercase_valid);
936- tcase_add_test(tc_core, test_hip_string_is_digit_valid);
937- tcase_add_test(tc_core, test_hip_string_is_digit_null);
938- tcase_add_test(tc_core, test_hip_string_is_digit_empty);
939- tcase_add_test(tc_core, test_base64_encode_valid);
940- tcase_add_test(tc_core, test_base64_encode_null_buf);
941- tcase_add_test(tc_core, test_base64_encode_empty_buf);
942+ tcase_add_test(tc_core, test_hip_in6_ntop_valid);
943+ tcase_add_test(tc_core, test_hip_in6_ntop_null_addr);
944+ tcase_add_test(tc_core, test_hip_in6_ntop_null_buf);
945+ tcase_add_test(tc_core, test_hip_convert_string_to_address_valid);
946+ tcase_add_test(tc_core, test_hip_convert_string_to_address_null_str);
947+ tcase_add_test(tc_core, test_hip_convert_string_to_address_null_addr);
948+ tcase_add_test(tc_core, test_hip_convert_string_to_address_invalid);
949 suite_add_tcase(s, tc_core);
950
951 return s;

Subscribers

People subscribed via source and target branches

to all changes: