Re: CVS commit: src/lib/libc/rpc

[Chuck Silvers <chuq%chuq.com@localhost>]

hi,

part of this change is:

+#ifdef _REENTRANT
+                       if (errno == EINTR) {
+                               sigset_t rmask;
+                               if (sigpending(&rmask) == -1) {
+                                       cu->cu_error.re_errno = errno;
+                                       release_fd_lock(cu->cu_fd, mask);
+                                       return cu->cu_error.re_status =
+                                           RPC_SYSTEMERROR;
+                               }
+                               (void)sigsuspend(&rmask);
+                       }
+#endif


why did you add this part?  if we got EINTR, then the signal was already
delivered and thus won't be pending anymore.  plus, why would it make sense
to set the mask to exactly the signals that are already pending?  this would
require receiving a different signal than any already pending.  if the
intent was to unmask any pending signals, this seems wrong too, because
the pending signal could have been masked by the application before calling
the RPC code, and the library shouldn't allow signals that the application
has intentionally blocked.  I think this block should just be removed.

also, it looks like sigpending() needs a threaded version in libpthread too.

-Chuck


On Fri, Sep 09, 2005 at 03:40:49PM +0000, Christos Zoulas wrote:
> 
> Module Name:  src
> Committed By: christos
> Date:         Fri Sep  9 15:40:49 UTC 2005
> 
> Modified Files:
>       src/lib/libc/rpc: clnt_dg.c
> 
> Log Message:
> PR/31264: Mark Davies: rup not interruptable
> The cause of this is that in the re-entrant case we block all signals until
> we timeout. Convert this to use pollts and then grab the pending signals
> and sigsuspend them.
> XXX: We should really convert this to use kqueue, like FreeBSD did.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -r1.15 -r1.16 src/lib/libc/rpc/clnt_dg.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.