Re: [PATCH v2] ptsname_r: don't leak uninitialized memory

[Roland McGrath <roland at hack dot frob dot com>]

Any user-visible bug needs a bugzilla report filed and BZ# tag in log entries.

> +	* sysdeps/unix/sysv/linux/ptsname.c (__ptsname_internal): return
> +	errno if the TIOCGPTN ioctl fails with an error different than
> +	EINVAL.

Capitalize the sentence.

> --- /dev/null
> +++ b/login/tst-ptsname.c
> @@ -0,0 +1,138 @@

A new file needs the standard header at the top: a descriptive one-line
comment, and the copyright header.

> +static int
> +test_ok (int fd)

It looks simple enough to consolidate all these into a single function that
takes (int fd, char *buf, size_t buflen, int expected_error).

> +      printf ("ptsname_r(): expected: return = %d\n", 0);

"()" is not part of a function's name.  
Don't use it in messages (or comments).

> +  int ret, err;
> +  char *buf = NULL;
> +
> +  ret = ptsname_r (fd, buf, 1);
> +  err = errno;

Don't pre-declare.  Always use C99 style and define at first assignment.

> +  const char file[] = "./ptsname-einval";

Just use a macro for the string constant, not a local variable (which does
a useless copy instead of just passing the rodata string constant).

This function too can use the common function above (twice) for most of its
guts.

> +  /* Then try with a terminal device which is not a master. */
> +  ret = ptsname_r (0, buf, sizeof (buf));

Don't assume stdin is a terminal.  (Incidentally, when using stdin is
appropriate, using STDIN_FILENO from <unistd.h> is good implicit
documentation of intent.)  You can open /dev/tty, but the test should
not report failure (perhaps report untestable if we have that now)
if the open fails.

> +int
> +main (void)

Use test-skeleton.c.

> +      close(fd);

Space before paren.


Thanks,
Roland