Re: ptsname_r

2011-11-10 Thread Christopher Faylor
On Tue, Nov 08, 2011 at 01:04:18AM -0500, Christopher Faylor wrote:
On Mon, Nov 07, 2011 at 01:59:40PM -0700, Eric Blake wrote:
On 11/07/2011 01:46 PM, Eric Blake wrote:
 Thanks. Also, even with your patches of today, ptsname() is still not
 thread-safe; should we be sticking that in a thread-local buffer rather
 than in static storage, similar to how other functions like strerror()
 are thread-safe?

I didn't tackle that,


 Also, should we have an efault handler in syscalls.cc ptsname_r(),
 similar to ttyname_r(), so as to gracefully reject invalid buffers
 rather than faulting?

but I had this additional code in my sandbox right before your commit 
hit CVS; should I add a ChangeLog and make it a formal patch submission?

No thanks.  Except for the Copyright change in stdlib.h, there is nothing that
I think should go in.  On inspection, openpty() needs some work.

Actually, Corinna has added your patch to posix.sgml.  I forgot that gnu
functions were mentioned there too.

cgf


Re: ptsname_r

2011-11-07 Thread Eric Blake

On 11/07/2011 01:46 PM, Eric Blake wrote:

Thanks. Also, even with your patches of today, ptsname() is still not
thread-safe; should we be sticking that in a thread-local buffer rather
than in static storage, similar to how other functions like strerror()
are thread-safe?


I didn't tackle that,



Also, should we have an efault handler in syscalls.cc ptsname_r(),
similar to ttyname_r(), so as to gracefully reject invalid buffers
rather than faulting?


but I had this additional code in my sandbox right before your commit 
hit CVS; should I add a ChangeLog and make it a formal patch submission?


diff --git a/winsup/cygwin/include/cygwin/stdlib.h 
b/winsup/cygwin/include/cygwin/stdlib.h

index d2dfe4c..20358ef 100644
--- a/winsup/cygwin/include/cygwin/stdlib.h
+++ b/winsup/cygwin/include/cygwin/stdlib.h
@@ -1,6 +1,6 @@
 /* stdlib.h

-   Copyright 2005, 2006, 2007, 2008, 2009 Red Hat Inc.
+   Copyright 2005, 2006, 2007, 2008, 2009, 2011 Red Hat Inc.

 This file is part of Cygwin.

diff --git a/winsup/cygwin/libc/bsdlib.cc b/winsup/cygwin/libc/bsdlib.cc
index 3b6e7e4..c4398d3 100644
--- a/winsup/cygwin/libc/bsdlib.cc
+++ b/winsup/cygwin/libc/bsdlib.cc
@@ -108,7 +108,7 @@ openpty (int *amaster, int *aslave, char *name, 
const struct termios *termp,

 {
   grantpt (master);
   unlockpt (master);
-  strcpy (pts, ptsname (master));
+  ptsname_r (master, pts, sizeof pts);
   revoke (pts);
   if ((slave = open (pts, O_RDWR | O_NOCTTY)) = 0)
{
diff --git a/winsup/cygwin/posix.sgml b/winsup/cygwin/posix.sgml
index ef27fde..5c510ed 100644
--- a/winsup/cygwin/posix.sgml
+++ b/winsup/cygwin/posix.sgml
@@ -1131,6 +1131,7 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008)./para
 pow10f
 ppoll
 pthread_getattr_np
+ptsname_r
 removexattr
 setxattr
 strchrnul
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 514d458..68dec59 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -2913,16 +2913,24 @@ ptsname (int fd)
 extern C int
 ptsname_r (int fd, char *buf, size_t buflen)
 {
-  if (!buf)
+  int ret = 0;
+  myfault efault;
+  if (efault.faulted ())
+ret = EFAULT;
+  else
 {
-  set_errno (EINVAL);
-  return EINVAL;
+  cygheap_fdget cfd (fd, true);
+  if (cfd  0)
+   ret = EBADF;
+  else if (!buf)
+ret = EINVAL;
+  else
+   ret = cfd-ptsname_r (buf, buflen);
 }
-
-  cygheap_fdget cfd (fd);
-  if (cfd  0)
-return 0;
-  return cfd-ptsname_r (buf, buflen);
+  if (ret)
+set_errno (ret);
+  debug_printf (returning %d pts: %s, ret, ret ? NULL : buf);
+  return ret;
 }

 static int __stdcall

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org


Re: ptsname_r

2011-11-07 Thread Christopher Faylor
On Mon, Nov 07, 2011 at 01:59:40PM -0700, Eric Blake wrote:
On 11/07/2011 01:46 PM, Eric Blake wrote:
 Thanks. Also, even with your patches of today, ptsname() is still not
 thread-safe; should we be sticking that in a thread-local buffer rather
 than in static storage, similar to how other functions like strerror()
 are thread-safe?

I didn't tackle that,


 Also, should we have an efault handler in syscalls.cc ptsname_r(),
 similar to ttyname_r(), so as to gracefully reject invalid buffers
 rather than faulting?

but I had this additional code in my sandbox right before your commit 
hit CVS; should I add a ChangeLog and make it a formal patch submission?

No thanks.  Except for the Copyright change in stdlib.h, there is nothing that
I think should go in.  On inspection, openpty() needs some work.

cgf