[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-tech
Subject:    diff xmalloc
From:       "Ted Unangst" <tedu () tedunangst ! com>
Date:       2015-02-19 15:10:01
Message-ID: ebd8f4c2.93f851dd.cf0d460a () tedunangst ! com
[Download RAW message or body]

For after release, but since I was looking in the file...

Some of the checks in xmalloc.c are excessive. I think we've moved past the
mid-90s era when coping with various busted libcs was important for ssh (where
this file originated). I'll note that the error messages aren't particuarly
helpful in many cases (NULL) and even incorrect (xfree calls err() but doesn't
set errno).

Gathering up NULL checks in one place makes sense; imposing some sort of
runtime portability check doesn't.


Index: diffdir.c
===================================================================
RCS file: /cvs/src/usr.bin/diff/diffdir.c,v
retrieving revision 1.43
diff -u -p -r1.43 diffdir.c
--- diffdir.c	16 Jan 2015 06:40:07 -0000	1.43
+++ diffdir.c	19 Feb 2015 15:02:26 -0000
@@ -166,13 +166,13 @@ diffdir(char *p1, char *p2, int flags)
 closem:
 	if (dirp1 != NULL) {
 		for (dp1 = dirp1; dp1 < edp1; dp1++)
-			xfree(*dp1);
-		xfree(dirp1);
+			free(*dp1);
+		free(dirp1);
 	}
 	if (dirp2 != NULL) {
 		for (dp2 = dirp2; dp2 < edp2; dp2++)
-			xfree(*dp2);
-		xfree(dirp2);
+			free(*dp2);
+		free(dirp2);
 	}
 }
 
Index: diffreg.c
===================================================================
RCS file: /cvs/src/usr.bin/diff/diffreg.c,v
retrieving revision 1.85
diff -u -p -r1.85 diffreg.c
--- diffreg.c	5 Feb 2015 12:59:57 -0000	1.85
+++ diffreg.c	19 Feb 2015 15:02:47 -0000
@@ -385,7 +385,7 @@ diffreg(char *file1, char *file2, int fl
 		case -1:
 			warnx("No more processes");
 			status |= 2;
-			xfree(header);
+			free(header);
 			rval = D_ERROR;
 			goto closem;
 		case 0:
@@ -406,7 +406,7 @@ diffreg(char *file1, char *file2, int fl
 			}
 			close(pfd[0]);
 			rewind(stdout);
-			xfree(header);
+			free(header);
 		}
 	}
 	prepare(0, f1, stb1.st_size, flags);
@@ -429,13 +429,13 @@ diffreg(char *file1, char *file2, int fl
 	clistlen = 100;
 	clist = xcalloc(clistlen, sizeof(*clist));
 	i = stone(class, slen[0], member, klist, flags);
-	xfree(member);
-	xfree(class);
+	free(member);
+	free(class);
 
 	J = xrealloc(J, len[0] + 2, sizeof(*J));
 	unravel(klist[i]);
-	xfree(clist);
-	xfree(klist);
+	free(clist);
+	free(klist);
 
 	ixold = xrealloc(ixold, len[0] + 2, sizeof(*ixold));
 	ixnew = xrealloc(ixnew, len[1] + 2, sizeof(*ixnew));
@@ -899,7 +899,7 @@ unsort(struct line *f, int l, int *b)
 		a[f[i].serial] = f[i].value;
 	for (i = 1; i <= l; i++)
 		b[i] = a[i];
-	xfree(a);
+	free(a);
 }
 
 static int
@@ -1006,7 +1006,7 @@ ignoreline(char *line)
 	int ret;
 
 	ret = regexec(&ignore_re, line, 0, NULL, 0);
-	xfree(line);
+	free(line);
 	return (ret == 0);	/* if it matched, it should be ignored. */
 }
 
Index: xmalloc.c
===================================================================
RCS file: /cvs/src/usr.bin/diff/xmalloc.c,v
retrieving revision 1.5
diff -u -p -r1.5 xmalloc.c
--- xmalloc.c	5 Feb 2015 12:59:57 -0000	1.5
+++ xmalloc.c	19 Feb 2015 15:05:00 -0000
@@ -27,11 +27,9 @@ xmalloc(size_t size)
 {
 	void *ptr;
 
-	if (size == 0)
-		errx(2, "xmalloc: zero size");
 	ptr = malloc(size);
 	if (ptr == NULL)
-		err(2, NULL);
+		err(2, "xmalloc %zu", size);
 	return ptr;
 }
 
@@ -40,14 +38,10 @@ xcalloc(size_t nmemb, size_t size)
 {
 	void *ptr;
 
-	if (size == 0 || nmemb == 0)
-		errx(2, "xcalloc: zero size");
-	if (SIZE_MAX / nmemb < size)
-		errx(2, "xcalloc: nmemb * size > SIZE_MAX");
 	ptr = calloc(nmemb, size);
 	if (ptr == NULL)
-		errx(2, "xcalloc: out of memory (allocating %lu bytes)",
-		    (u_long)(size * nmemb));
+		err(2, "xcalloc: out of memory (allocating %zu*%zu bytes)",
+		    nmemb, size);
 	return ptr;
 }
 
@@ -55,38 +49,21 @@ void *
 xrealloc(void *ptr, size_t nmemb, size_t size)
 {
 	void *new_ptr;
-	size_t new_size = nmemb * size;
 
-	if (new_size == 0)
-		errx(2, "xrealloc: zero size");
-	if (SIZE_MAX / nmemb < size)
-		errx(2, "xrealloc: nmemb * size > SIZE_MAX");
-	if (ptr == NULL)
-		new_ptr = malloc(new_size);
-	else
-		new_ptr = realloc(ptr, new_size);
+	new_ptr = reallocarray(ptr, nmemb, size);
 	if (new_ptr == NULL)
-		err(2, NULL);
+		err(2, "xrealloc %zu*%zu", nmemb, size);
 	return new_ptr;
 }
 
-void
-xfree(void *ptr)
-{
-	if (ptr == NULL)
-		err(2, NULL);
-	free(ptr);
-}
-
 char *
 xstrdup(const char *str)
 {
-	size_t len;
 	char *cp;
 
-	len = strlen(str) + 1;
-	cp = xmalloc(len);
-	strlcpy(cp, str, len);
+	cp = strdup(str);
+	if (cp == NULL)
+		err(2, "xstrdup");
 	return cp;
 }
 
@@ -101,7 +78,7 @@ xasprintf(char **ret, const char *fmt, .
 	va_end(ap);
 
 	if (i < 0 || *ret == NULL)
-		err(2, NULL);
+		err(2, "xasprintf");
 
 	return (i);
 }

[prev in list] [next in list] [prev in thread] [next in thread]