strip's error handling appears to be questionable. The ERROR() define
has a continue statement at the end, yet in use there are statements
behind it. gcc2 does not warn about this.
strip also tries to loop over multiple files despite previous errors and
returns an error code in the end. There are gotos sprinkled throughout
to support this. I don't think that is helpful at all and have changed
it to immediately terminate.
This patch survived a hp300 make build
Index: strip.c
===================================================================
RCS file: /home/vcs/cvs/openbsd/src/usr.bin/strip/strip.c,v
retrieving revision 1.29
diff -u -p -r1.29 strip.c
--- strip.c 1 Jun 2010 21:44:39 -0000 1.29
+++ strip.c 2 Mar 2012 01:52:26 -0000
@@ -78,7 +78,7 @@ main(int argc, char *argv[])
EXEC *ep;
struct stat sb;
int (*sfcn)(const char *, int, EXEC *, struct stat *, off_t *);
- int ch, errors;
+ int ch;
char *fn, *ofile = NULL;
off_t newsize;
@@ -109,32 +109,24 @@ main(int argc, char *argv[])
if (ofile != NULL && argc > 1)
usage();
- errors = 0;
-#define ERROR(x) errors |= 1; warnx("%s: %s", fn, strerror(x));
continue;
while ((fn = *argv++)) {
if (ofile) {
char buf[8192];
- ssize_t wn;
- size_t rn;
+ ssize_t wn, rn;
off_t off;
int wfd;
- if ((fd = open(fn, O_RDONLY)) < 0) {
- ERROR(errno);
- break;
- }
- if ((wfd = open(ofile, O_RDWR|O_CREAT)) < 0) {
- ERROR(errno);
- break;
- }
+ if ((fd = open(fn, O_RDONLY)) < 0)
+ err(1, "open: %s", fn);
+
+ if ((wfd = open(ofile, O_RDWR|O_CREAT)) < 0)
+ err(1, "open: %s", ofile);
do {
rn = read(fd, buf, sizeof buf);
- if (rn == (ssize_t)-1) {
- int save_errno = errno;
-
- unlink(ofile);
- ERROR(save_errno);
- exit(errors);
+ if (rn == -1) {
+ warn("read: %s", fn);
+ (void) unlink(ofile);
+ exit(1);
}
if (rn == 0)
break;
@@ -142,41 +134,35 @@ main(int argc, char *argv[])
off = 0;
while (rn - off > 0) {
wn = write(wfd, buf + off, rn - off);
- if (wn == (ssize_t)-1) {
- int save_errno = errno;
-
- unlink(ofile);
- ERROR(save_errno);
- exit(errors);
+ if (wn == -1) {
+ warn("write: %s", ofile);
+ (void) unlink(ofile);
+ exit(1);
}
off += wn;
}
} while (rn > 0);
fn = ofile;
- close(fd);
+ (void) close(fd);
fd = wfd;
} else if ((fd = open(fn, O_RDWR)) < 0) {
- ERROR(errno);
- }
- if (fstat(fd, &sb)) {
- (void)close(fd);
- ERROR(errno);
- }
- if (sb.st_size < sizeof(EXEC)) {
- (void)close(fd);
- ERROR(EFTYPE);
- }
- if ((ep = (EXEC *)mmap(NULL, sb.st_size, PROT_READ|PROT_WRITE,
- MAP_SHARED, fd, (off_t)0)) == MAP_FAILED) {
- (void)close(fd);
- ERROR(errno);
+ err(1, "open: %s", fn);
}
- if (BAD_OBJECT(*ep)) {
- munmap((caddr_t)ep, sb.st_size);
- (void)close(fd);
- ERROR(EFTYPE);
+
+ if (fstat(fd, &sb))
+ err(1, "fstat: %s", fn);
+
+ if (sb.st_size < sizeof(EXEC))
+ errx(1, "%s: %s", fn, strerror(EFTYPE));
+
+ if ((ep = mmap(NULL, sb.st_size, PROT_READ|PROT_WRITE,
+ MAP_SHARED, fd, (off_t)0)) == MAP_FAILED) {
+ err(1, "mmap: %s", fn);
}
+
+ if (BAD_OBJECT(*ep))
+ errx(1, "%s: %s", fn, strerror(EFTYPE));
/*
* Since we're dealing with an mmap there, we have to convert
* once for dealing with data in memory, and a second time
@@ -184,19 +170,16 @@ main(int argc, char *argv[])
*/
fix_header_order(ep);
newsize = 0;
- errors |= sfcn(fn, fd, ep, &sb, &newsize);
+ if (sfcn(fn, fd, ep, &sb, &newsize) != 0)
+ exit(1);
fix_header_order(ep);
munmap((caddr_t)ep, sb.st_size);
- if (newsize && ftruncate(fd, newsize)) {
- warn("%s", fn);
- errors = 1;
- }
- if (close(fd)) {
- ERROR(errno);
- }
+ if (newsize && ftruncate(fd, newsize))
+ err(1, "truncate: %s", fn);
+ if (close(fd))
+ err(1, "close: %s", fn);
}
-#undef ERROR
- exit(errors);
+ exit(0);
}
int
@@ -271,7 +254,6 @@ s_stab(const char *fn, int fd, EXEC *ep,
int mid;
NLIST *symbase;
unsigned int *mapping=0;
- int error=1;
unsigned int nsyms;
struct relocation_info *reloc_base;
unsigned int i, j;
@@ -280,10 +262,8 @@ s_stab(const char *fn, int fd, EXEC *ep,
if (ep->a_syms == 0)
return 0;
- if (N_SYMOFF(*ep) >= sp->st_size) {
- warnx("%s: bad symbol table", fn);
- return 1;
- }
+ if (N_SYMOFF(*ep) >= sp->st_size)
+ errx(1, "%s: bad symbol table", fn);
mid = N_GETMID(*ep);
@@ -301,59 +281,49 @@ s_stab(const char *fn, int fd, EXEC *ep,
*/
strbase = (char *)ep + N_STROFF(*ep);
allocsize = fix_32_order(*(u_long *)strbase, mid);
- if ((nstrbase = malloc((u_int) allocsize)) == NULL) {
- warnx("%s", strerror(ENOMEM));
- goto end;
- }
+ if ((nstrbase = malloc((u_int) allocsize)) == NULL)
+ err(1, "malloc");
+
nstr = nstrbase + sizeof(u_long);
/* okay, so we also need to keep symbol numbers for relocations. */
nsyms = ep->a_syms/ sizeof(NLIST);
used = calloc(nsyms, 1);
- if (!used) {
- warnx("%s", strerror(ENOMEM));
- goto end;
- }
+ if (!used)
+ err(1, "calloc");
+
mapping = calloc(nsyms, sizeof(unsigned int));
- if (!mapping) {
- warnx("%s", strerror(ENOMEM));
- goto end;
- }
+ if (!mapping)
+ err(1, "calloc");
- if ((ep->a_trsize || ep->a_drsize) && byte_sex(mid) != BYTE_ORDER) {
- warnx("%s: cross-stripping not supported", fn);
- goto end;
- }
+ if ((ep->a_trsize || ep->a_drsize) && byte_sex(mid) != BYTE_ORDER)
+ errx(1, "%s: cross-stripping not supported", fn);
/* first check the relocations for used symbols, and mark them */
/* text */
reloc_base = (struct relocation_info *) ((char *)ep + N_TRELOFF(*ep));
- if (N_TRELOFF(*ep) + ep->a_trsize > sp->st_size) {
- warnx("%s: bad text relocation", fn);
- goto end;
- }
+ if (N_TRELOFF(*ep) + ep->a_trsize > sp->st_size)
+ errx(1,"%s: bad text relocation", fn);
+
for (i = 0; i < ep->a_trsize / sizeof(struct relocation_info); i++) {
if (!reloc_base[i].r_extern)
continue;
- if (reloc_base[i].r_symbolnum > nsyms) {
- warnx("%s: bad symbol number in text relocation", fn);
- goto end;
- }
+ if (reloc_base[i].r_symbolnum > nsyms)
+ errx(1, "%s: bad symbol number in text relocation", fn);
+
used[reloc_base[i].r_symbolnum] = 1;
}
/* data */
reloc_base = (struct relocation_info *) ((char *)ep + N_DRELOFF(*ep));
- if (N_DRELOFF(*ep) + ep->a_drsize > sp->st_size) {
- warnx("%s: bad data relocation", fn);
- goto end;
- }
+ if (N_DRELOFF(*ep) + ep->a_drsize > sp->st_size)
+ errx(1, "%s: bad data relocation", fn);
+
for (i = 0; i < ep->a_drsize / sizeof(struct relocation_info); i++) {
if (!reloc_base[i].r_extern)
continue;
- if (reloc_base[i].r_symbolnum > nsyms) {
- warnx("%s: bad symbol number in data relocation", fn);
- goto end;
- }
+ if (reloc_base[i].r_symbolnum > nsyms)
+ errx(1, "%s: bad symbol number in data relocation", fn);
+
used[reloc_base[i].r_symbolnum] = 1;
}
@@ -378,10 +348,9 @@ s_stab(const char *fn, int fd, EXEC *ep,
}
len = strlen(p) + 1;
mapping[i] = j++;
- if (N_STROFF(*ep) + sym->strx + len > sp->st_size) {
- warnx("%s: bad symbol table", fn);
- goto end;
- }
+ if (N_STROFF(*ep) + sym->strx + len > sp->st_size)
+ errx(1, "%s: bad symbol table", fn);
+
bcopy(p, nstr, len);
nstr += len;
fix_nlist_order(nsym++, mid);
@@ -416,8 +385,6 @@ s_stab(const char *fn, int fd, EXEC *ep,
* at the address past the last symbol entry.
*/
bcopy(nstrbase, (void *)nsym, len);
- error = 0;
-end:
free(nstrbase);
free(used);
free(mapping);
@@ -425,7 +392,7 @@ end:
/* Truncate to the current length. */
*sz = (char *)nsym + len - (char *)ep;
- return error;
+ return 0;
}
void
