Re: [PATCH,HURD] hurd: compliance fixes for ptsname_r

[Pino Toscano]

Hi,

Alle venerdì 20 luglio 2012, Roland McGrath ha scritto:
> > Ok, I see that its `buf' argument is marked nonnull. I added that
> > check because I saw the gnulib test for it explicitly checking
> > that ptsname_r(fd, NULL, 0) would be properly failing with EINVAL
> > (and the man page even explicitly mention that return value,
> > unlike with basically most of the other functions). Should gnulib
> > do that check only on Linux, then?
> 
> Well, everybody's wrong.  The libc manual never said that you can
> pass NULL and expect not to crash, and the man page was IMHO wrong
> to document it that way.  The other implementations never should
> have checked for NULL, but they have done so for a long time. 
> gnulib never should have passed NULL to this function and IMHO it
> should be fixed not to do so. But given the history, the least of
> avaialble evils is to make the Hurd implementation consistent with
> the others and do the check.

(few months later... I forgot I sent this patch, so I'm bring it again.)

I updated the patch; is it okay to commit, or should I bring back the 
buf==NULL check?

-- 
Pino Toscano

Hurd: fixes for ptsname and ptsname_r

ptsname_r on failure returns the value that is also set as errno; furthermore,
add more checks to it:
- set errno and return it on __term_get_peername failure
- set errno to ERANGE other than returning it
- change the type of PEERNAME to string_t, and check its length with __strnlen

In ptsname:
- change the type of PEERNAME to string_t
- do not set errno manually, since ptsname_r has set it already

2012-11-19  Pino Toscano  <toscano.p...@tiscali.it>

	* sysdeps/mach/hurd/ptsname.c (ptsname): Change the type of PEERNAME to
	string_t.  Do not manually set errno.
	(__ptsname_r): Change the type of PEERNAME to string_t, and check its
	length with __strnlen.  Make sure to both set errno and return it on
	failure.
--- a/sysdeps/mach/hurd/ptsname.c
+++ b/sysdeps/mach/hurd/ptsname.c
@@ -29,12 +29,10 @@
 char *
 ptsname (int fd)
 {
-  static char peername[1024];  /* XXX */
+  static string_t peername;
   error_t err;
 
   err = __ptsname_r (fd, peername, sizeof (peername));
-  if (err)
-    __set_errno (err);
 
   return err ? NULL : peername;
 }
@@ -46,17 +44,19 @@ ptsname (int fd)
 int
 __ptsname_r (int fd, char *buf, size_t buflen)
 {
-  char peername[1024];  /* XXX */
+  string_t peername;
   size_t len;
   error_t err;
 
-  peername[0] = '\0';
   if (err = HURD_DPORT_USE (fd, __term_get_peername (port, peername)))
-    return _hurd_fd_error (fd, err);
+    return __hurd_dfail (fd, err), errno;
 
-  len = strlen (peername) + 1;
+  len = __strnlen (peername, sizeof peername - 1) + 1;
   if (len > buflen)
-    return ERANGE;
+    {
+      errno = ERANGE;
+      return ERANGE;
+    }
 
   memcpy (buf, peername, len);
   return 0;

signature.asc
Description: This is a digitally signed message part.