Date: Thu, 27 May 2010 10:05:47 GMT From: Garrett Cooper <gcooper@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 178863 for review Message-ID: <201005271005.o4RA5lqr023676@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201005271005.o4RA5lqr023676>