Thank you for the diff.

> I looked for more instances of the pattern that lead to reading one byte
> before an allocated buffer in which(1) when PATH begins with "/:". I
> found only one, in the function csexists() in usr.bin/mg/cscope.c.

> +     while ((dir = strsep(&path, ":")) != NULL) {
> +             if (*dir== '\0')
> +                     *dir = '.';
> 
> -          dlen = strlen(dir);
> -               while (dir[dlen-1] == '/')
> -                       dir[--dlen] = '\0';     /* strip trailing '/' */
> +             dlen = strlen(dir);
> +             while (dlen > 0 && dir[dlen-1] == '/')
> +                     dir[--dlen] = '\0';     /* strip trailing '/' */

dlen could never be zero as we are replacing dir[0] with '.' if
it's an empty field but that has another problem of wrong strlen(3)
values due to improper NUL termination. The simple fix is to skip
empty fields in PATH which I committed.

> 
> While at it, I replaced the manual length check before snprintf() with a
> check of the return value, and I replaced the space indents in the
> csexists() function with tabs. (The rest of the file uses tabs.)
> If that makes the diff inconvenient to check, I'll resubmit it without
> the indent change.

These changes are committed. It's a good idea to split diffs where
possible for ease of readability and review.

Reply via email to