From owner-p4-projects@FreeBSD.ORG Tue May 18 05:04:34 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id E2C6E1065672; Tue, 18 May 2010 05:04:33 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 984CC106566B for ; Tue, 18 May 2010 05:04:33 +0000 (UTC) (envelope-from gcooper@FreeBSD.org) Received: from repoman.freebsd.org (unknown [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 8445E8FC0C for ; Tue, 18 May 2010 05:04:33 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id o4I54X5W069944 for ; Tue, 18 May 2010 05:04:33 GMT (envelope-from gcooper@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id o4I54Xem069942 for perforce@freebsd.org; Tue, 18 May 2010 05:04:33 GMT (envelope-from gcooper@FreeBSD.org) Date: Tue, 18 May 2010 05:04:33 GMT Message-Id: <201005180504.o4I54Xem069942@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to gcooper@FreeBSD.org using -f From: Garrett Cooper To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 178428 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 May 2010 05:04:34 -0000 http://p4web.freebsd.org/@@178428?ac=10 Change 178428 by gcooper@gcooper-bayonetta on 2010/05/18 05:04:16 Remove all user printable code and refactor things so that this file (at least) functions more like a library. Affected files ... .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/file.c#16 edit Differences ... ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/file.c#16 (text+ko) ==== @@ -197,23 +197,30 @@ { struct stat sb; char *contents = NULL; - int fd = -1; + int fd; + int serrno; + + errno = 0; + + if ((fd = open(fname, O_RDONLY)) >= 0 && fstat(fd, &sb) == 0) { + + if ((contents = (char *)malloc(sb.st_size + 1)) != NULL && + read(fd, contents, sb.st_size) != sb.st_size) { + serrno = errno; + /* free can screw up errno's value. */ + free(contents); + contents = NULL; + errno = serrno; + } - if (stat(fname, &sb) == -1) { - warn("%s: can't stat '%s'", __func__, fname); - } else { - contents = (char *)malloc(sb.st_size + 1); - fd = open(fname, O_RDONLY, 0); - if (fd == -1) - warn("%s: unable to open '%s' for reading", - __func__, fname); - else if (read(fd, contents, sb.st_size) != sb.st_size) - warn("%s: short read on '%s' - did not get %lld bytes", - __func__, fname, (long long) sb.st_size); } - if (0 <= fd) + if (0 <= fd) { + serrno = errno; close(fd); + if (serrno != 0) + errno = serrno; + } if (contents != NULL) contents[sb.st_size] = '\0'; @@ -268,23 +275,23 @@ FILE *fp = NULL; off_t written_len = -1; size_t len; + int serrno; errno = 0; fp = fopen(name, "w"); - if (fp == NULL) - warn("%s: cannot fopen '%s' for writing", __func__, name); - else { + if (fp != NULL) { + len = strlen(str); written_len = fwrite(str, 1, len, fp); - if ((len-written_len) != 0) - warn("%s: short fwrite on '%s', tried to write %ld " - "bytes", __func__, name, (long) len); + + if (fp != NULL) { + serrno = errno; + (void) fclose(fp); + if (serrno != 0) + errno = serrno; + } - if (fp != NULL) - if (fclose(fp) != 0) - warn("%s: failed to close file: '%s'", - __func__, name); } return (size_t) (errno == 0 && written_len > 0 ? written_len : -1); @@ -310,9 +317,14 @@ snprintf(to, FILENAME_MAX, "%s/%s", tdir, fname); - if (rename(from, to) == 0 || vsystem("/bin/mv %s %s", from, to) == 0) + if (rename(from, to) == 0 || vsystem("/bin/mv %s %s", from, to) == 0) { + /* + * Avoid confusing errno values because one of the calls + * passed. + */ + errno = 0; rc = 0; - else + } else rc = -1; return rc; @@ -338,6 +350,7 @@ struct stat sb; char *buf = NULL; int fd; + int serrno; if ((fd = unpack_to_fd(pkg, file)) != -1) { @@ -350,15 +363,23 @@ */ buf = malloc(sb.st_size); if (buf != NULL) { - if (read(fd, buf, sb.st_size) != sb.st_size) + + if (read(fd, buf, sb.st_size) != sb.st_size) { free(buf); + buf = NULL; + } + } } } - if (fd != -1) + if (0 <= fd) { + serrno = errno; close(fd); + if (serrno != 0) + errno = serrno; + } return buf; @@ -381,27 +402,13 @@ Boolean extract_whole_archive = FALSE; const char *entry_pathname = NULL; const char *error = NULL; - const char *pkg_name_humanized; int archive_fd = -1, r; + errno = 0; + if (file_expr == NULL || strcmp("*", file_expr) == 0) extract_whole_archive = TRUE; - if (pkg == NULL || strcmp(pkg, "-") == 0) - pkg_name_humanized = "(stdin)"; - else - pkg_name_humanized = pkg; - - if (Verbose) { - if (extract_whole_archive) - printf("%s: %s - will extract whole archive\n", - pkg_name_humanized, __func__); - else - printf("%s: %s - will extract files that match " - "expression: %s\n", - pkg_name_humanized, __func__, file_expr); - } - if ((archive = archive_read_new()) != NULL) { if (archive_read_support_compression_all(archive) @@ -424,13 +431,8 @@ if (archive_fd != -1 || archive == NULL) ; /* archive(3) failed to open the file descriptor. */ else if (archive_read_open_fd(archive, archive_fd, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { - + ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) error = archive_error_string(archive); - warnx("%s: unable to open the package from %s: %s", - __func__, pkg_name_humanized, error); - - } else while (error == NULL && (r = archive_read_next_header(archive, &archive_entry)) == @@ -445,28 +447,21 @@ r = archive_read_extract(archive, archive_entry, EXTRACT_ARCHIVE_FLAGS); - if (r == ARCHIVE_OK) { - if (Verbose) - printf("X - %s\n", - entry_pathname); - } else { + if (r != ARCHIVE_OK) error = archive_error_string(archive); - warnx("%s: extraction for %s failed: " - "%s", __func__, pkg_name_humanized, - error); - } - } else - if (Verbose) - printf("S - %s\n", entry_pathname); + } } +#if 0 + /* + * This should be stored in a global buffer or something similar that's + * retrievable via pkg_error or something of that flavor. + */ if (errno != 0) error = strerror(errno); - if (error != NULL) - warnx("%s: unpacking package - %s - failed: %s", - __func__, pkg_name_humanized, error); +#endif if (archive != NULL) archive_read_finish(archive); @@ -495,20 +490,11 @@ const char *entry_pathname = NULL; const char *error = NULL; - const char *pkg_name_humanized; - int fd = -1; /* int fd = -1; */ int archive_fd = -1, r; - if (pkg == NULL || strcmp(pkg, "-") == 0) - pkg_name_humanized = "(stdin)"; - else - pkg_name_humanized = pkg; - - if (Verbose) - printf("%s: will extract %s from %s\n", - __func__, file, pkg_name_humanized); + errno = 0; if ((archive = archive_read_new()) != NULL) { @@ -532,11 +518,8 @@ if (archive_fd != -1 || archive == NULL) ; /* archive(3) failed to open the file descriptor. */ else if (archive_read_open_fd(archive, archive_fd, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) error = archive_error_string(archive); - warnx("%s: unable to open the package from %s: %s", - __func__, pkg_name_humanized, error); - } else while (error == NULL && found_match == FALSE && (r = archive_read_next_header(archive, &archive_entry)) == @@ -556,29 +539,23 @@ r = archive_read_extract(archive, archive_entry, EXTRACT_ARCHIVE_FLAGS); - if (r == ARCHIVE_OK) { - if (Verbose) - printf("X - %s\n", - entry_pathname); + if (r == ARCHIVE_OK) fd = open(entry_pathname, O_RDONLY); - } else { + else error = archive_error_string(archive); - warnx("%s: extraction for %s failed: " - "%s", __func__, pkg_name_humanized, - error); - } - } else - if (Verbose) - printf("S - %s\n", entry_pathname); + } } +#if 0 + /* + * This should be stored in a global buffer or something similar that's + * retrievable via pkg_error or something of that flavor. + */ if (errno != 0) error = strerror(errno); - if (error != NULL) - warnx("%s: unable to read the file - %s - from package: %s: " - "%s", __func__, file, pkg_name_humanized, error); +#endif if (archive != NULL) archive_read_finish(archive); @@ -607,7 +584,7 @@ char *cp, scratch[FILENAME_MAX * 2]; int l; - while (*fmt && max > 0) { + while (*fmt != '\0' && max > 0) { if (*fmt == '%') { switch (*++fmt) { case 'F': @@ -648,7 +625,12 @@ --max; break; } - ++fmt; + /* + * Avoid cases where malformed strings can be like: 'foobar%'. -- + * this can lead to not so awesome problems, like buffer overruns. + */ + if (fmt != '\0') + fmt++; } else { *buf++ = *fmt++;