Re: svn commit: r366781 - in head: include lib/libc/stdlib

2020-10-18 Thread Xin Li via svn-src-all
Hi,

Thanks very much for the feedback.  I have created a new change for
review at https://reviews.freebsd.org/D26845 , could you please take a
look and let me know if it's satisfactory?

On 10/16/20 11:53 PM, Konstantin Belousov wrote:
[...]>> +int ptsname_r(int, char *, size_t);
> This declaration appears in the __XSI_VISIBLE block, but I do not see the
> function description in the IEEE Std 1003.1™-2017 text (base issue 7).
> 
> Review mentioned that the function is scheduled for issue 8, but shouldn't
> it put under BSD_VISIBLE until specification is finalized ?

Good point, fixed in the proposed change.

[...]
>> Modified: head/lib/libc/stdlib/Symbol.map
>> ==
>> --- head/lib/libc/stdlib/Symbol.map  Sat Oct 17 01:06:04 2020
>> (r366780)
>> +++ head/lib/libc/stdlib/Symbol.map  Sat Oct 17 04:14:38 2020
>> (r366781)
>> @@ -125,6 +125,7 @@ FBSD_1.6 {
>>  qsort_s;
>>  rand;
>>  srand;
>> +ptsname_r;
> This is unsorted now.

Fixed in the proposed change.

[...]
>> +{
>> +static char pt_slave[sizeof _PATH_DEV + SPECNAMELEN];
> We usually write sizeof(_PATH_DEV).

This was from previous code so I leave it as-is, but fixed in the
proposed change.

>> +
>> +if (ptsname_r(fildes, pt_slave, sizeof(pt_slave)) == 0)
> Since ptsname_r is non-standard and exported, userspace is allowed to
> interpose the symbol, which would break ptsname().

Thanks for pointing it out -- I should have paid more attention here.
Fixed in the proposed change.

>> +return (pt_slave);
>> +else
> 'else' is redundand.

Removed else and used a straight return instead.

Cheers,
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r366781 - in head: include lib/libc/stdlib

2020-10-17 Thread Konstantin Belousov
On Sat, Oct 17, 2020 at 04:14:39AM +, Xin LI wrote:
> Author: delphij
> Date: Sat Oct 17 04:14:38 2020
> New Revision: 366781
> URL: https://svnweb.freebsd.org/changeset/base/366781
> 
> Log:
>   Implement ptsname_r.
>   
>   MFC after:  2 weeks
>   PR: 250062
>   Reviewed by:jilles, 0mp, Ray 
>   Differential Revision:  https://reviews.freebsd.org/D26647
> 
> Modified:
>   head/include/stdlib.h
>   head/lib/libc/stdlib/Makefile.inc
>   head/lib/libc/stdlib/Symbol.map
>   head/lib/libc/stdlib/ptsname.3
>   head/lib/libc/stdlib/ptsname.c
> 
> Modified: head/include/stdlib.h
> ==
> --- head/include/stdlib.h Sat Oct 17 01:06:04 2020(r366780)
> +++ head/include/stdlib.h Sat Oct 17 04:14:38 2020(r366781)
> @@ -225,6 +225,7 @@ long   mrand48(void);
>  long  nrand48(unsigned short[3]);
>  int   posix_openpt(int);
>  char *ptsname(int);
> +int   ptsname_r(int, char *, size_t);
This declaration appears in the __XSI_VISIBLE block, but I do not see the
function description in the IEEE Std 1003.1™-2017 text (base issue 7).

Review mentioned that the function is scheduled for issue 8, but shouldn't
it put under BSD_VISIBLE until specification is finalized ?

>  int   putenv(char *);
>  long  random(void);
>  unsigned short
> 
> Modified: head/lib/libc/stdlib/Makefile.inc
> ==
> --- head/lib/libc/stdlib/Makefile.inc Sat Oct 17 01:06:04 2020
> (r366780)
> +++ head/lib/libc/stdlib/Makefile.inc Sat Oct 17 04:14:38 2020
> (r366781)
> @@ -50,7 +50,7 @@ MLINKS+=hcreate.3 hdestroy.3 hcreate.3 hsearch.3
>  MLINKS+=hcreate.3 hcreate_r.3 hcreate.3 hdestroy_r.3 hcreate.3 hsearch_r.3
>  MLINKS+=insque.3 remque.3
>  MLINKS+=lsearch.3 lfind.3
> -MLINKS+=ptsname.3 grantpt.3 ptsname.3 unlockpt.3
> +MLINKS+=ptsname.3 grantpt.3 ptsname.3 ptsname_r.3 ptsname.3 unlockpt.3
>  MLINKS+=qsort.3 heapsort.3 qsort.3 mergesort.3 qsort.3 qsort_r.3 \
>   qsort.3 qsort_s.3
>  MLINKS+=rand.3 rand_r.3 rand.3 srand.3
> 
> Modified: head/lib/libc/stdlib/Symbol.map
> ==
> --- head/lib/libc/stdlib/Symbol.map   Sat Oct 17 01:06:04 2020
> (r366780)
> +++ head/lib/libc/stdlib/Symbol.map   Sat Oct 17 04:14:38 2020
> (r366781)
> @@ -125,6 +125,7 @@ FBSD_1.6 {
>   qsort_s;
>   rand;
>   srand;
> + ptsname_r;
This is unsorted now.

>  };
>  
>  FBSDprivate_1.0 {
> 
> Modified: head/lib/libc/stdlib/ptsname.3
> ==
> --- head/lib/libc/stdlib/ptsname.3Sat Oct 17 01:06:04 2020
> (r366780)
> +++ head/lib/libc/stdlib/ptsname.3Sat Oct 17 04:14:38 2020
> (r366781)
> @@ -31,12 +31,13 @@
>  .\"
>  .\" $FreeBSD$
>  .\"
> -.Dd August 20, 2008
> +.Dd October 17, 2020
>  .Dt PTSNAME 3
>  .Os
>  .Sh NAME
>  .Nm grantpt ,
>  .Nm ptsname ,
> +.Nm ptsname_r ,
>  .Nm unlockpt
>  .Nd pseudo-terminal access functions
>  .Sh LIBRARY
> @@ -47,6 +48,8 @@
>  .Fn grantpt "int fildes"
>  .Ft "char *"
>  .Fn ptsname "int fildes"
> +.Ft "int"
> +.Fn ptsname_r "int fildes" "char *buffer" "size_t buflen"
>  .Ft int
>  .Fn unlockpt "int fildes"
>  .Sh DESCRIPTION
> @@ -87,12 +90,23 @@ and
>  have been called.
>  .Pp
>  The
> +.Fn ptsname_r
> +function is the thread-safe version of
> +.Fn ptsname .
> +The caller must provide storage for the results of the full pathname of
> +the slave device in the
> +.Fa buffer
> +and
> +.Fa bufsize
> +arguments.
> +.Pp
> +The
>  .Fn unlockpt
>  function clears the lock held on the pseudo-terminal pair
>  for the master device specified with
>  .Fa fildes .
>  .Sh RETURN VALUES
> -.Rv -std grantpt unlockpt
> +.Rv -std grantpt ptsname_r unlockpt
>  .Pp
>  The
>  .Fn ptsname
> @@ -103,7 +117,8 @@ pointer is returned.
>  .Sh ERRORS
>  The
>  .Fn grantpt ,
> -.Fn ptsname
> +.Fn ptsname ,
> +.Fn ptsname_r
>  and
>  .Fn unlockpt
>  functions may fail and set
> @@ -116,6 +131,16 @@ is not a valid open file descriptor.
>  .It Bq Er EINVAL
>  .Fa fildes
>  is not a master pseudo-terminal device.
> +.El
> +.Pp
> +In addition, the
> +.Fn ptsname_r
> +function may set
> +.Va errno
> +to:
> +.Bl -tag -width Er
> +.It Bq Er ERANGE
> +The buffer was too small.
>  .El
>  .Pp
>  In addition, the
> 
> Modified: head/lib/libc/stdlib/ptsname.c
> ==
> --- head/lib/libc/stdlib/ptsname.cSat Oct 17 01:06:04 2020
> (r366780)
> +++ head/lib/libc/stdlib/ptsname.cSat Oct 17 04:14:38 2020
> (r366781)
> @@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "un-namespace.h"
>  
>  /*
> @@ -71,23 +72,46 @@ __strong_reference(__isptmaster, grantpt);
>  __strong_reference(__isptmaster, unlockpt);
>  
>  /*
> - * ptsname(): 

svn commit: r366781 - in head: include lib/libc/stdlib

2020-10-16 Thread Xin LI
Author: delphij
Date: Sat Oct 17 04:14:38 2020
New Revision: 366781
URL: https://svnweb.freebsd.org/changeset/base/366781

Log:
  Implement ptsname_r.
  
  MFC after:2 weeks
  PR:   250062
  Reviewed by:  jilles, 0mp, Ray 
  Differential Revision:https://reviews.freebsd.org/D26647

Modified:
  head/include/stdlib.h
  head/lib/libc/stdlib/Makefile.inc
  head/lib/libc/stdlib/Symbol.map
  head/lib/libc/stdlib/ptsname.3
  head/lib/libc/stdlib/ptsname.c

Modified: head/include/stdlib.h
==
--- head/include/stdlib.h   Sat Oct 17 01:06:04 2020(r366780)
+++ head/include/stdlib.h   Sat Oct 17 04:14:38 2020(r366781)
@@ -225,6 +225,7 @@ long mrand48(void);
 longnrand48(unsigned short[3]);
 int posix_openpt(int);
 char   *ptsname(int);
+int ptsname_r(int, char *, size_t);
 int putenv(char *);
 longrandom(void);
 unsigned short

Modified: head/lib/libc/stdlib/Makefile.inc
==
--- head/lib/libc/stdlib/Makefile.inc   Sat Oct 17 01:06:04 2020
(r366780)
+++ head/lib/libc/stdlib/Makefile.inc   Sat Oct 17 04:14:38 2020
(r366781)
@@ -50,7 +50,7 @@ MLINKS+=hcreate.3 hdestroy.3 hcreate.3 hsearch.3
 MLINKS+=hcreate.3 hcreate_r.3 hcreate.3 hdestroy_r.3 hcreate.3 hsearch_r.3
 MLINKS+=insque.3 remque.3
 MLINKS+=lsearch.3 lfind.3
-MLINKS+=ptsname.3 grantpt.3 ptsname.3 unlockpt.3
+MLINKS+=ptsname.3 grantpt.3 ptsname.3 ptsname_r.3 ptsname.3 unlockpt.3
 MLINKS+=qsort.3 heapsort.3 qsort.3 mergesort.3 qsort.3 qsort_r.3 \
qsort.3 qsort_s.3
 MLINKS+=rand.3 rand_r.3 rand.3 srand.3

Modified: head/lib/libc/stdlib/Symbol.map
==
--- head/lib/libc/stdlib/Symbol.map Sat Oct 17 01:06:04 2020
(r366780)
+++ head/lib/libc/stdlib/Symbol.map Sat Oct 17 04:14:38 2020
(r366781)
@@ -125,6 +125,7 @@ FBSD_1.6 {
qsort_s;
rand;
srand;
+   ptsname_r;
 };
 
 FBSDprivate_1.0 {

Modified: head/lib/libc/stdlib/ptsname.3
==
--- head/lib/libc/stdlib/ptsname.3  Sat Oct 17 01:06:04 2020
(r366780)
+++ head/lib/libc/stdlib/ptsname.3  Sat Oct 17 04:14:38 2020
(r366781)
@@ -31,12 +31,13 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd August 20, 2008
+.Dd October 17, 2020
 .Dt PTSNAME 3
 .Os
 .Sh NAME
 .Nm grantpt ,
 .Nm ptsname ,
+.Nm ptsname_r ,
 .Nm unlockpt
 .Nd pseudo-terminal access functions
 .Sh LIBRARY
@@ -47,6 +48,8 @@
 .Fn grantpt "int fildes"
 .Ft "char *"
 .Fn ptsname "int fildes"
+.Ft "int"
+.Fn ptsname_r "int fildes" "char *buffer" "size_t buflen"
 .Ft int
 .Fn unlockpt "int fildes"
 .Sh DESCRIPTION
@@ -87,12 +90,23 @@ and
 have been called.
 .Pp
 The
+.Fn ptsname_r
+function is the thread-safe version of
+.Fn ptsname .
+The caller must provide storage for the results of the full pathname of
+the slave device in the
+.Fa buffer
+and
+.Fa bufsize
+arguments.
+.Pp
+The
 .Fn unlockpt
 function clears the lock held on the pseudo-terminal pair
 for the master device specified with
 .Fa fildes .
 .Sh RETURN VALUES
-.Rv -std grantpt unlockpt
+.Rv -std grantpt ptsname_r unlockpt
 .Pp
 The
 .Fn ptsname
@@ -103,7 +117,8 @@ pointer is returned.
 .Sh ERRORS
 The
 .Fn grantpt ,
-.Fn ptsname
+.Fn ptsname ,
+.Fn ptsname_r
 and
 .Fn unlockpt
 functions may fail and set
@@ -116,6 +131,16 @@ is not a valid open file descriptor.
 .It Bq Er EINVAL
 .Fa fildes
 is not a master pseudo-terminal device.
+.El
+.Pp
+In addition, the
+.Fn ptsname_r
+function may set
+.Va errno
+to:
+.Bl -tag -width Er
+.It Bq Er ERANGE
+The buffer was too small.
 .El
 .Pp
 In addition, the

Modified: head/lib/libc/stdlib/ptsname.c
==
--- head/lib/libc/stdlib/ptsname.c  Sat Oct 17 01:06:04 2020
(r366780)
+++ head/lib/libc/stdlib/ptsname.c  Sat Oct 17 04:14:38 2020
(r366781)
@@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
 #include 
 #include 
 #include 
+#include 
 #include "un-namespace.h"
 
 /*
@@ -71,23 +72,46 @@ __strong_reference(__isptmaster, grantpt);
 __strong_reference(__isptmaster, unlockpt);
 
 /*
- * ptsname():  return the pathname of the slave pseudo-terminal device
- * associated with the specified master.
+ * ptsname_r(): return the pathname of the slave pseudo-terminal device
+ *  associated with the specified master.
  */
-char *
-ptsname(int fildes)
+int
+ptsname_r(int fildes, char *buffer, size_t buflen)
 {
-   static char pt_slave[sizeof _PATH_DEV + SPECNAMELEN] = _PATH_DEV;
-   char *ret = NULL;
 
+   if (buflen <= sizeof(_PATH_DEV)) {
+   errno = ERANGE;
+   return (-1);
+   }
+
/* Make sure fildes points to a master device. */
if (__isptmaster(fildes) != 0)