Clean up format_hex_ex()
Cherry-pick of commit 29404010 from master, slightly reworked to match the
2.3 codebase (no flags inside space_break here), and pulled in the new
static_assert() fallback we also have in master now.
Fix a potential null-pointer dereference, and make the code a bit more
readable while doing so.
The NULL dereference could not be triggered, because the current code
never called format_hex_ex() with maxouput == 0 and separator == NULL.
But it's nicer to not depend on that.
Our use of int vs size_t for lengths needs some attention too, but I'm
not pulling that into this patch. Instead I decided to just make the
(previously existing) assumption that INT_MAX <= SIZE_MAX explicit by
adding a static_assert().
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1480352107-19652-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13269.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger authored on 2016/11/29 01:55:07Showing 2 changed files
| ... | ... |
@@ -400,9 +400,13 @@ format_hex_ex (const uint8_t *data, int size, int maxoutput, |
| 400 | 400 |
int space_break, const char* separator, |
| 401 | 401 |
struct gc_arena *gc) |
| 402 | 402 |
{
|
| 403 |
- struct buffer out = alloc_buf_gc (maxoutput ? maxoutput : |
|
| 404 |
- ((size * 2) + (size / space_break) * (int) strlen (separator) + 2), |
|
| 405 |
- gc); |
|
| 403 |
+ const size_t separator_len = separator ? strlen (separator) : 0; |
|
| 404 |
+ static_assert (INT_MAX <= SIZE_MAX, "Code assumes INT_MAX <= SIZE_MAX"); |
|
| 405 |
+ const size_t out_len = maxoutput > 0 ? maxoutput : |
|
| 406 |
+ ((size * 2) + ((size / space_break) * separator_len) + 2); |
|
| 407 |
+ |
|
| 408 |
+ struct buffer out = alloc_buf_gc (out_len, gc); |
|
| 409 |
+ |
|
| 406 | 410 |
int i; |
| 407 | 411 |
for (i = 0; i < size; ++i) |
| 408 | 412 |
{
|
| ... | ... |
@@ -217,6 +217,14 @@ FILE *msg_fp(const unsigned int flags); |
| 217 | 217 |
void assert_failed (const char *filename, int line, const char *condition) |
| 218 | 218 |
__attribute__((__noreturn__)); |
| 219 | 219 |
|
| 220 |
+/* Poor-man's static_assert() for when not supplied by assert.h, taken from |
|
| 221 |
+ * Linux's sys/cdefs.h under GPLv2 */ |
|
| 222 |
+#ifndef static_assert |
|
| 223 |
+#define static_assert(expr, diagnostic) \ |
|
| 224 |
+ extern int (*__OpenVPN_static_assert_function (void)) \ |
|
| 225 |
+ [!!sizeof (struct { int __error_if_negative: (expr) ? 2 : -1; })]
|
|
| 226 |
+#endif |
|
| 227 |
+ |
|
| 220 | 228 |
#ifdef ENABLE_DEBUG |
| 221 | 229 |
void crash (void); /* force a segfault (debugging only) */ |
| 222 | 230 |
#endif |