---
 include/names.h |  3 ++-
 include/utils.h |  4 ++++
 lib/names.c     | 69 +++++++++++++++++++++++++++++++++++++++------------------
 lib/utils.c     | 26 ++++++++++++++++++++++
 tc/tc_util.c    | 19 ++++++++++++----
 5 files changed, 95 insertions(+), 26 deletions(-)

include/names.h +2 -1

index 4123d0b..6fed581 100644
@@ -16,7 +16,8 @@ struct db_names {
 	int max;
 };
 
-struct db_names *db_names_alloc(const char *path);
+struct db_names *db_names_alloc(void);
+int db_names_load(struct db_names *db, const char *path);
 void db_names_free(struct db_names *db);
 
 char *id_to_name(struct db_names *db, int id, char *name);

include/utils.h +4 -0

index 9151c4f..4bb1bf0 100644
@@ -172,4 +172,8 @@ extern int do_each_netns(int (*func)(char *nsname, void *arg), void *arg,
 
 char *int_to_str(int val, char *buf);
 
+void *xmalloc(size_t size);
+void xfree(void *ptr);
+char *xstrdup(char *str);
+
 #endif /* __UTILS_H__ */

lib/names.c +48 ~~-21~~

136 lines not on this page · raw message

lib/utils.c +26 -0

index 9cda268..d119a69 100644
@@ -904,3 +904,29 @@ char *int_to_str(int val, char *buf)
 	sprintf(buf, "%d", val);
 	return buf;
 }
+
+void *xmalloc(size_t size)
+{
+	void *ptr = malloc(size);
+	if (!ptr)
+		fprintf(stderr, "xmalloc: Out of memory\n");
+
+	return ptr;
+}
+
+void xfree(void *ptr)
+{
+	if (!ptr)
+		fprintf(stderr, "xfree: ptr is NULL\n");
+	else
+		free(ptr);
+}
+
+char *xstrdup(char *str)
+{
+	char *ptr = strdup(str);
+	if (!ptr)
+		fprintf(stderr, "xstrdup: Out of memory\n");
+
+	return ptr;
+}

tc/tc_util.c +15 -4

index feae439..5213e9e 100644
@@ -21,6 +21,7 @@
 #include <arpa/inet.h>
 #include <string.h>
 #include <math.h>
+#include <errno.h>
 
 #include "utils.h"
 #include "names.h"

@@ -33,15 +34,25 @@
 
 static struct db_names *cls_names = NULL;
 
-#define NAMES_DB "/etc/iproute2/cls_names"
+#define NAMES_DB "/etc/iproute2/tc_cls"
 
 int cls_names_init(char *path)
 {
-	cls_names = db_names_alloc(path ?: NAMES_DB);
-	if (!cls_names) {
-		fprintf(stderr, "Error while opening class names file\n");
+	int ret = -1;
+
+	cls_names = db_names_alloc();
+	if (!cls_names)
+		return -1;
+
+	ret = db_names_load(cls_names, path ?: NAMES_DB);
+	if (ret == -ENOENT && path) {
+		fprintf(stderr, "Can't open class names file: %s\n", path);
 		return -1;
 	}
+	if (ret) {
+		db_names_free(cls_names);
+		cls_names = NULL;
+	}
 
 	return 0;
 }

Signature · 1 line

2.3.1

Daniel Borkmann

2015-03-17 14:32 UTC #1 lore raw

Quoted 13 lines · Vadim Kochan

From: Vadim Kochan <[email protected]>
Added the following changes in one patch:
1) Split db_names_init into alloc & load funcs
2) Added checks for out of memory with new alloc helpers
3) Ignore if there is no default class names file
3) Changed default class names path cls_names -> tc_cls
Signed-off-by Vadim Kochan <[email protected]>

...

Quoted 24 lines

lib/utils.c +16 -0

index 9cda268..d119a69 100644
@@ -904,3 +904,29 @@ char *int_to_str(int val, char *buf)
  	sprintf(buf, "%d", val);
  	return buf;
  }
+
+void *xmalloc(size_t size)
+{
+	void *ptr = malloc(size);
+	if (!ptr)
+		fprintf(stderr, "xmalloc: Out of memory\n");
+
+	return ptr;
+}
+
+void xfree(void *ptr)
+{
+	if (!ptr)
+		fprintf(stderr, "xfree: ptr is NULL\n");
+	else
+		free(ptr);

lib/utils.c char *int_to_str(int val, char *buf) · line 932

Quoted 10 lines

lib/utils.c +10 -0

+}
+
+char *xstrdup(char *str)
+{
+	char *ptr = strdup(str);
+	if (!ptr)
+		fprintf(stderr, "xstrdup: Out of memory\n");
+
+	return ptr;
+}

When you add xmalloc and friends, the expectation is rather different then what you've implemented : i.e. xmalloc dies in case the allocation fails, so that xmalloc() et al users don't have to care about the NULL check.

Simple example:

void *xmalloc(size_t size)

{
	void *ptr;

	ptr = malloc(size);
	if (unlikely(ptr == NULL))
		panic("xmalloc: out of memory (allocating %zu bytes)\n",
		      size);

	return ptr;
}

For a more elaborate example, you can also have a look at git, e.g. wrapper.c +84, but a simpler form is sufficient here.

I think there are two possible ways forward: either use normal malloc() and check for NULL, or you could add in a different set xmalloc() et al, and convert also other users from iproute2 that don't have an error recovery, iow do abort() or the like.

Other than that, would be good if you split up this patch into a set.