On Wed, Sep 19, 2018 at 09:19:22AM -0600, Todd C. Miller wrote:
> When getcwd() fails, the stat buffer 'swd' is used uninitialized
> by the else clause. Since it is used in both clauses we should
> perform the stat before the if().
>
> However, fixing this causes 'cp' to be unitialized in some case so
> initialize cp to NULL and move the "cp == NULL" check out of the
> first if() clause now that it can be true in either case.
>
> Found by clang analyzer.
>
> - todd
I had to stare at this one for a while, but overall I like the patch.
OK miko@.
>
> Index: bin/csh/dir.c
> ===================================================================
> RCS file: /cvs/src/bin/csh/dir.c,v
> retrieving revision 1.21
> diff -u -p -u -r1.21 dir.c
> --- bin/csh/dir.c 26 Dec 2015 13:48:38 -0000 1.21
> +++ bin/csh/dir.c 19 Sep 2018 15:14:43 -0000
> @@ -64,7 +64,7 @@ void
> dinit(Char *hp)
> {
> char *tcp;
> - Char *cp;
> + Char *cp = NULL;
> struct directory *dp;
> char path[PATH_MAX];
> static const char emsg[] = "csh: Trying to start from \"%s\"\n";
> @@ -75,45 +75,45 @@ dinit(Char *hp)
> (void) fprintf(csherr, "csh: %s\n", strerror(errno));
> if (hp && *hp) {
> tcp = short2str(hp);
> - if (chdir(tcp) == -1)
> - cp = NULL;
> - else
> + if (chdir(tcp) == 0)
> cp = hp;
> (void) fprintf(csherr, emsg, vis_str(hp));
> }
> - else
> - cp = NULL;
> - if (cp == NULL) {
> - (void) fprintf(csherr, emsg, "/");
> - if (chdir("/") == -1)
> - /* I am not even try to print an error message! */
> - xexit(1);
> - cp = SAVE("/");
> - }
> }
> else {
> struct stat swd, shp;
>
> - /*
> - * See if $HOME is the working directory we got and use that
> - */
> - if (hp && *hp &&
> - stat(tcp, &swd) != -1 && stat(short2str(hp), &shp) != -1 &&
> - swd.st_dev == shp.st_dev && swd.st_ino == shp.st_ino)
> - cp = hp;
> - else {
> - char *cwd;
> -
> + if (stat(tcp, &swd) == -1) {
> + (void) fprintf(csherr, "csh: %s: %s\n", tcp, strerror(errno));
> + } else {
> /*
> - * use PWD if we have it (for subshells)
> + * See if $HOME is the working directory we got and use that
> */
> - if ((cwd = getenv("PWD")) != NULL) {
> - if (stat(cwd, &shp) != -1 && swd.st_dev == shp.st_dev &&
> - swd.st_ino == shp.st_ino)
> - tcp = cwd;
> + if (hp && *hp && stat(short2str(hp), &shp) != -1 &&
> + swd.st_dev == shp.st_dev && swd.st_ino == shp.st_ino)
> + cp = hp;
> + else {
> + char *cwd;
> +
> + /*
> + * use PWD if we have it (for subshells)
> + */
> + if ((cwd = getenv("PWD")) != NULL) {
> + if (stat(cwd, &shp) != -1 && swd.st_dev == shp.st_dev &&
> + swd.st_ino == shp.st_ino)
> + tcp = cwd;
> + }
> + cp = dcanon(SAVE(tcp), STRNULL);
> }
> - cp = dcanon(SAVE(tcp), STRNULL);
> }
> + }
> +
> + if (cp == NULL) {
> + (void) fprintf(csherr, emsg, "/");
> + if (chdir("/") == -1)
> + /* I am not even try to print an error message! */
> + xexit(1);
> + cp = SAVE("/");
> }
>
> dp = xcalloc(1, sizeof(struct directory));
>
