[bug-gnulib] Re: New getlogin_r module

[Derek Price]

Bruno Haible wrote:

>Derek Price wrote:
>  
>
>>This is what I installed.
>>    
>>
>
>Note that lib/ and m4/ have ChangeLogs of their own. Only the top-level
>and modules/* modifications go into the top-level ChangeLog. Including
>the update of MODULES.html.sh (adding one line for each new module).
>  
>

Thank you.  I will use these ChangeLogs in the future.

>Three further remarks:
>
>- In the include file, you use size_t without including <stddef.h>. Won't
>  work on platforms where unistd.h doesn't exist.
>  
>

Looks like Paul got ahead of me and installed this first.

>- The documentation: When a programmer wants to know what the function does,
>  he doesn't need/want to know how the function is implemented. Patch
>  appended.
>  
>

Installed.

>- The code in getlogin_r.c: Why do you save and restore errno? It's useless,
>  since 1. POSIX doesn't say that errno is preserved (generally it's the
>  caller's duty to save errno if he wants to), 
>

Section 7.1.4 of the C89 spec states, "The value of errno ... is never
set to zero by any library function."  I am assuming that getlogin_r and
probably most other GNULIB functions should act like library functions
when possible?

>2. you don't restore it
>  before "return errno;".
>  
>

I restore errno only when it was not set.  Otherwise, I am intentionally
allowing errno to be set.  This isn't POSIX compliant to the point of
not having side-effects not specified explicitly by POSIX, but this is
what the glibc getlogin_r function is doing and what it is documented to
do and I chose to emulate that behavior.

I was about to install the attached patch, which is more careful about
always restoring errno when it would have been set to zero, when Paul
beat me to the punch.  I am not adverse to always restoring errno and
will leave things as Paul set them if you wish, but I still object to
never restoring it.  (I would rather have no side-effects involving
errno rather than always setting it, sometimes to zero.)  Is there some
reason we shouldn't comply with the C89 errno specification for library
functions?

I've also added getlogin_r to the POSIX.2001 support section of
MODULES.html.sh.

2005-05-25  Derek Price  <address@hidden>

       * MODULES.html.sh: Add getlogin_r to POSIX.2001 support section.

2005-05-25  Bruno Haible  <address@hidden>
            Derek Price  <address@hidden>

        * lib/getlogin_r.h: Simplify API documentation.
        * lib/getlogin_r.c: Be more careful about restoring errno when
reset.

Regards,

Derek

Index: lib/getlogin_r.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/getlogin_r.c,v
retrieving revision 1.1
diff -u -p -r1.1 getlogin_r.c
--- lib/getlogin_r.c    25 May 2005 14:21:20 -0000      1.1
+++ lib/getlogin_r.c    25 May 2005 18:43:32 -0000
@@ -39,6 +39,7 @@ getlogin_r (char *name, size_t size)
 {
   char *n;
   int save_errno = errno;
+  int retval = -1;
 
   errno = 0;
   n = getlogin ();
@@ -48,12 +49,14 @@ getlogin_r (char *name, size_t size)
       if (nlen < size)
         {
           memcpy (name, n, nlen + 1);
-          return 0;
+          retval =  0;
         }
-      errno = ERANGE;
+      else
+        retval = ERANGE;
     }
 
-  if (errno) return errno;
-  errno = save_errno;
-  return -1;
+  if (errno) retval = errno;
+  else errno = save_errno;
+
+  return retval;
 }