This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Alle sabato 28 aprile 2012, Roland McGrath ha scritto:
> Compliance with what?  Is there a standard that specifies ptsname_r?

It is a Linux-specific function is implemented by GNU libc and other 
systems.

> We don't use implicit boolean coercions, we use (buf != NULL).

Noted.

> But unless there is a standard requiring that you diagnose a
> null pointer argument somehow, then it's actually better that
> it just crash.

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?

Attached the updated patch, also with the hits of the other reviews.

Thanks,
-- 
Pino Toscano
hurd: fixes for 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
Also, change the type of `peername' to string_t, and check its length with
__strnlen.

In ptsname do not set errno manually, since ptsname_r has set it already.

2012-04-28  Pino Toscano  <toscano.pino@tiscali.it>

	* sysdeps/mach/hurd/ptsname.c (ptsname): Do not manually set errno.
	(__ptsname_r): Set errno and return it on __term_get_peername failure.
	Set errno if the buffer is too short.  Make PEERNAME a string_t,
	and check its length with __strnlen.
--- a/sysdeps/mach/hurd/ptsname.c
+++ b/sysdeps/mach/hurd/ptsname.c
@@ -35,8 +35,6 @@
   error_t err;
 
   err = __ptsname_r (fd, peername, sizeof (peername));
-  if (err)
-    __set_errno (err);
 
   return err ? NULL : peername;
 }
@@ -47,17 +45,19 @@
 int
 __ptsname_internal (int fd, char *buf, size_t buflen, struct stat64 *stp)
 {
-  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;

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]