| Message ID | 20170829134515.9345-1-christian.brauner@ubuntu.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2,v4] openpty: close slave pty fd on error | expand |
On Aug 29 2017, Christian Brauner <christian.brauner@ubuntu.com> wrote: > @@ -129,12 +124,17 @@ openpty (int *amaster, int *aslave, char *name, > if (name != NULL) > strcpy (name, buf); > > + ret = 0; > + > + on_error: > + close (master); > + > + if (slave != -1) > + close(slave); > + You don't want to close the fds on success. Andreas.
On Tue, Aug 29, 2017 at 04:00:26PM +0200, Andreas Schwab wrote: > On Aug 29 2017, Christian Brauner <christian.brauner@ubuntu.com> wrote: > > > @@ -129,12 +124,17 @@ openpty (int *amaster, int *aslave, char *name, > > if (name != NULL) > > strcpy (name, buf); > > > > + ret = 0; > > + > > + on_error: > > + close (master); > > + > > + if (slave != -1) > > + close(slave); > > + > > You don't want to close the fds on success. Sorry, I was inatentive. > > Andreas. > > -- > Andreas Schwab, SUSE Labs, schwab@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different."
diff --git a/ChangeLog b/ChangeLog index bc1cf94dc3..bc5fb8e27f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2017-08-26 Christian Brauner <christian.brauner@ubuntu.com> + + * login/openpty.c (openpty): Close slave pty fd and unify error handling. + 2017-08-25 H.J. Lu <hongjiu.lu@intel.com> * sysdeps/x86/cpu-features.h [__ASSEMBLER__] diff --git a/login/openpty.c b/login/openpty.c index 41ab0483e2..a7b1ab5dde 100644 --- a/login/openpty.c +++ b/login/openpty.c @@ -92,29 +92,24 @@ openpty (int *amaster, int *aslave, char *name, char _buf[512]; #endif char *buf = _buf; - int master, slave; + int master, ret = -1, slave = -1; master = getpt (); if (master == -1) return -1; if (grantpt (master)) - goto fail; + goto on_error; if (unlockpt (master)) - goto fail; + goto on_error; if (pts_name (master, &buf, sizeof (_buf))) - goto fail; + goto on_error; slave = open (buf, O_RDWR | O_NOCTTY); if (slave == -1) - { - if (buf != _buf) - free (buf); - - goto fail; - } + goto on_error; /* XXX Should we ignore errors here? */ if (termp) @@ -129,12 +124,17 @@ openpty (int *amaster, int *aslave, char *name, if (name != NULL) strcpy (name, buf); + ret = 0; + + on_error: + close (master); + + if (slave != -1) + close(slave); + if (buf != _buf) free (buf); - return 0; - fail: - close (master); - return -1; + return ret; } libutil_hidden_def (openpty)
When openpty() failed only the master fd was closed so far. Let's close the slave fd as well. Also, let's unify the error handling. Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- Changelog 2017-08-29: * Unify error handling: use a common function exit that frees everything that needs freeing. (@Florian) --- ChangeLog | 4 ++++ login/openpty.c | 28 ++++++++++++++-------------- 2 files changed, 18 insertions(+), 14 deletions(-)