[1/5] manual: Update to mention ENODEV for ttyname and ttyname_r

Message ID 20171012035321.22094-2-lukeshu@parabola.nu
State New
Headers show
Series
  • Fixup linux ttyname and ttyname_r [BZ #22145]
Related show

Commit Message

Luke Shumaker Oct. 12, 2017, 3:53 a.m.
Commit 15e9a4f3 by Christian Brauner <christian.brauner@canonical.com>
introduced ENODEV as a possible error condition for ttyname and ttyname_r.
The manual should mention this.
---
 ChangeLog            | 4 ++++
 manual/terminal.texi | 5 +++++
 2 files changed, 9 insertions(+)

Comments

Christian Brauner Oct. 12, 2017, 9:49 a.m. | #1
On Wed, Oct 11, 2017 at 11:53:17PM -0400, Luke Shumaker wrote:
> Commit 15e9a4f3 by Christian Brauner <christian.brauner@canonical.com>
> introduced ENODEV as a possible error condition for ttyname and ttyname_r.
> The manual should mention this.
> ---
> ChangeLog            | 4 ++++
> manual/terminal.texi | 5 +++++
> 2 files changed, 9 insertions(+)
> 
> diff --git a/ChangeLog b/ChangeLog
> index a3c6b0ab19..cc036f7b88 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-10-11  Luke Shumaker  <lukeshu@parabola.nu>
> +
> +	* manual/terminal.texi: Mention ENODEV for ttyname and ttyname_r.
> +
> 2017-10-10  Joseph Myers  <joseph@codesourcery.com>
> 
> * sysdeps/generic/libm-alias-double.h (libm_alias_double_other_r):
> diff --git a/manual/terminal.texi b/manual/terminal.texi
> index 4fef5045b8..f505e93af6 100644
> --- a/manual/terminal.texi
> +++ b/manual/terminal.texi
> @@ -109,6 +109,11 @@ The @var{filedes} is not associated with a terminal.
> @item ERANGE
> The buffer length @var{len} is too small to store the string to be
> returned.
> +
> +@item ENODEV
> +The @var{filedes} is valid, and is associated with a terminal, and
> +that terminal is a slave psuedo-terminal, but the associated file name
> +could not be determined.  This is a GNU extension.

Makes sense to me and is in line with a discussion I had with some of the
coreutils people.

@end table
@end deftypefun
Dmitry V. Levin Oct. 12, 2017, 6:44 p.m. | #2
On Wed, Oct 11, 2017 at 11:53:17PM -0400, Luke Shumaker wrote:
> Commit 15e9a4f3 by Christian Brauner <christian.brauner@canonical.com>
> introduced ENODEV as a possible error condition for ttyname and ttyname_r.
> The manual should mention this.
> ---
>  ChangeLog            | 4 ++++
>  manual/terminal.texi | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/ChangeLog b/ChangeLog
> index a3c6b0ab19..cc036f7b88 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-10-11  Luke Shumaker  <lukeshu@parabola.nu>
> +
> +	* manual/terminal.texi: Mention ENODEV for ttyname and ttyname_r.

I think this should be

	* manual/terminal.texi (Is It a Terminal): ...

> --- a/manual/terminal.texi
> +++ b/manual/terminal.texi
> @@ -109,6 +109,11 @@ The @var{filedes} is not associated with a terminal.
>  @item ERANGE
>  The buffer length @var{len} is too small to store the string to be
>  returned.
> +
> +@item ENODEV
> +The @var{filedes} is valid, and is associated with a terminal, and
> +that terminal is a slave psuedo-terminal, but the associated file name
> +could not be determined.  This is a GNU extension.

Something has to be done with the wording, e.g.

The @var{filedes} argument is a valid file descriptor associated with
a slave psuedo-terminal device, but the file name of that device could
not be determined.  This is a GNU extension.
Luke Shumaker Oct. 13, 2017, 12:17 a.m. | #3
On Wed, 11 Oct 2017 23:53:17 -0400,
Luke Shumaker wrote:
> +that terminal is a slave psuedo-terminal, but the associated file name
                            ^^^^^^

Someone mentioned in a private email to me (perhaps the just forgot to
CC the list) that I misspelled "pseudo" here.
Luke Shumaker Oct. 13, 2017, 12:33 a.m. | #4
On Thu, 12 Oct 2017 14:44:22 -0400,
Dmitry V. Levin wrote:
> > --- a/ChangeLog
> > +++ b/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2017-10-11  Luke Shumaker  <lukeshu@parabola.nu>
> > +
> > +	* manual/terminal.texi: Mention ENODEV for ttyname and ttyname_r.
> 
> I think this should be
> 
> 	* manual/terminal.texi (Is It a Terminal): ...

Sure.

On that note, am I otherwise doing the ChangeLog thing correctly?  One
of the wiki pages suggested that the ChangeLog entry should be part of
the commit messages, not part of the patch; looking through the
archives, both ways seem common.

> > --- a/manual/terminal.texi
> > +++ b/manual/terminal.texi
> > @@ -109,6 +109,11 @@ The @var{filedes} is not associated with a terminal.
> >  @item ERANGE
> >  The buffer length @var{len} is too small to store the string to be
> >  returned.
> > +
> > +@item ENODEV
> > +The @var{filedes} is valid, and is associated with a terminal, and
> > +that terminal is a slave psuedo-terminal, but the associated file name
> > +could not be determined.  This is a GNU extension.
> 
> Something has to be done with the wording, e.g.
> 
> The @var{filedes} argument is a valid file descriptor associated with
> a slave psuedo-terminal device, but the file name of that device could
> not be determined.  This is a GNU extension.

The wording is awkward, but the exact case of what ENODEV means is
awkward.  The problem I have with your wording is that it isn't as
immediately clear that "slave pseudo-terminal" is a special case of
"terminal", and that ENODEV does *not* apply to that more general
case.

Now, IMO, ENODEV probably *should* apply to the more general case of
"The @var{filedes} associated with a terminal device, but the file
name associated with that device could not be determined", but that's
not what it currently means, and it's beyond the scope of this
patchset to change that.

What about:

The @var{filedes} is associated with a terminal device that is a slave
pseudo-terminal, but the file name associated with that device could
not be determined.  This is a GNU extension.

Patch

diff --git a/ChangeLog b/ChangeLog
index a3c6b0ab19..cc036f7b88 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@ 
+2017-10-11  Luke Shumaker  <lukeshu@parabola.nu>
+
+	* manual/terminal.texi: Mention ENODEV for ttyname and ttyname_r.
+
 2017-10-10  Joseph Myers  <joseph@codesourcery.com>
 
 	* sysdeps/generic/libm-alias-double.h (libm_alias_double_other_r):
diff --git a/manual/terminal.texi b/manual/terminal.texi
index 4fef5045b8..f505e93af6 100644
--- a/manual/terminal.texi
+++ b/manual/terminal.texi
@@ -109,6 +109,11 @@  The @var{filedes} is not associated with a terminal.
 @item ERANGE
 The buffer length @var{len} is too small to store the string to be
 returned.
+
+@item ENODEV
+The @var{filedes} is valid, and is associated with a terminal, and
+that terminal is a slave psuedo-terminal, but the associated file name
+could not be determined.  This is a GNU extension.
 @end table
 @end deftypefun