From owner-p4-projects@FreeBSD.ORG Thu May 27 10:05:47 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id A95A01065790; Thu, 27 May 2010 10:05:47 +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 54A331065784 for ; Thu, 27 May 2010 10:05:47 +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 41CDD8FC14 for ; Thu, 27 May 2010 10:05:47 +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 o4RA5lrj023678 for ; Thu, 27 May 2010 10:05:47 GMT (envelope-from gcooper@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id o4RA5lqr023676 for perforce@freebsd.org; Thu, 27 May 2010 10:05:47 GMT (envelope-from gcooper@FreeBSD.org) Date: Thu, 27 May 2010 10:05:47 GMT Message-Id: <201005271005.o4RA5lqr023676@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 178863 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: Thu, 27 May 2010 10:05:47 -0000 http://p4web.freebsd.org/@@178863?ac=10 Change 178863 by gcooper@gcooper-bayonetta on 2010/05/27 10:05:35 1. Fix the linereading by using the proper variable. Properly cap the value for now by erroring out quickly noting the cause of failure, as opposed to silently failing. 2. Remove some debug strings. 3. Add some comments. Affected files ... .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/plist.c#6 edit Differences ... ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/plist.c#6 (text+ko) ==== @@ -196,11 +196,21 @@ int plist_cmd(const char *s, char **arg) { + /* XXX (gcooper): this can blow up really quickly with the recent + * modifications made to read_plist, provided a sufficiently large list. */ char cmd[FILENAME_MAX + 20]; /* 20 == fudge for max cmd len */ char *cp; const char *sp; - strlcpy(cmd, s, sizeof(cmd)); + /* + * FIXME (gcooper): this should be dynamic according to whatever's passed + * in. + */ + if (strlcpy(cmd, s, sizeof(cmd)) >= sizeof(cmd)) { + warnx("%s: line '%s' exceeds set limits", __func__, s); + errno = EINVAL; + return -1; + } str_lowercase(cmd); cp = cmd; sp = s; @@ -279,7 +289,7 @@ int major; int minor; int rc = -1; - off_t off; + off_t end_off; size_t len; pkg->fmtver_maj = 1; @@ -292,15 +302,11 @@ (contents_map = mmap(NULL, sb.st_size, PROT_READ, MAP_SHARED, fd, 0)) != NULL) { - off = sb.st_size; + end_off = sb.st_size; rc = 0; start = contents_map; - /* - * XXX (gcooper): BAD BAD BAD -- this can be longer than - * FILENAME_MAX - */ - while (rc == 0 && 0 < off) { + while (rc == 0 && 0 < end_off) { end = strchr(start, '\n'); /* No trailing newlines -- look for '\0'. */ @@ -308,7 +314,7 @@ end = strchr(start, '\0'); /* Don't forget we're eating newlines.. om nom nom... */ else - off--; + end_off--; /* * This is bad if this fails -- a non-NUL terminated * string is in our midst! @@ -323,12 +329,10 @@ strlcpy(cmd_buf, start, end-start+1); - warnx("cmd_buf: %s", cmd_buf); - len = strlen(cmd_buf); - off -= len; - - /* Trim off trailing whitespace. */ + end_off -= len; + + /* Trim end_off trailing whitespace. */ while (0 < len && isspace(cmd_buf[len])) cmd_buf[len--] = '\0'; @@ -344,7 +348,7 @@ /* A plist command directive */ if (rc == 0 && *start == CMD_CHAR) { - cmd = plist_cmd(start+1, &cp); + cmd = plist_cmd(cmd_buf+1, &cp); if (cmd == -1) { warnx("%s: unknown command '%s' " @@ -394,6 +398,11 @@ /* A file manifest item */ else if (rc == 0) cmd = PLIST_FILE; + + /* + * Winner, winner, chicken dinner.. we have a working + * command! + */ if (rc == 0) { add_plist(pkg, cmd, cp); @@ -404,6 +413,12 @@ } + /* + * XXX (gcooper): using more intelligent pointer + * arithmetic and proper NUL termination, there's no + * reason why this needs to be freed automatically in + * each iteration. + */ if (cmd_buf != NULL) { free (cmd_buf); cmd_buf = NULL; @@ -573,7 +588,6 @@ case PLIST_CWD: if (prefix == NULL) prefix = p->name; - warnx("prefix: %s", prefix); Where = (p->name == NULL) ? prefix : p->name; if (Verbose) printf("Change working directory to %s\n", Where);