[1/2,v4] openpty: close slave pty fd on error
Commit Message
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(-)
Comments
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."
@@ -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__]
@@ -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)