[OPEN-ILS-DEV] C nits: errno in _socket_handle_client_data()

[Mike Rylander]

On 5/29/07, Scott McKellar <mck9 at swbell.net> wrote:
> In socket_bundle.c, the _socket_handle_client_data function calls
> clr_fl().  In case of error, the calling function tests the value
> of errno, and if it is not EAGAIN, logs it in a message.
>
> By this time clr_fl() (in utils.c) has already written its own message
> to standard error, using fprintf().  Hence the errno reported in the
> calling function is the value left by fprintf().  What we presumably
> want, however, is the errno value from fcntl(), as called by clr_fl().

IMO, the stderr fprintf() calls seem wholly redundant -- it looks to
me like a leftover debugging statement.  As such, I'll go the other
direction from Scott's proposal and suggest that we remove them
entirely, and instead just add a log message to
libstack/osrf_prefork.c after it calls clr_fl().  That is the only
other call to the function that I can find in the source.

Bill, do you see any reason to keep those stderr fprintf()s,
especially in light of the fact that clr_fl() is used in
osrf_prefork.c (which probably shouldn't talk to stdout or stderr)?

--miker

>
> If fprintf() does not encounter an error, it will likely leave errno
> unchanged, but it doesn't have to.  The C89 standard allows a library
> function to change the value of errno even in the absence of an error,
> provided that the Standard does not document the use of errno in the
> description of the library function.  In the case of fprintf(), the C89
> Standard does not document the use of errno.  I don't know about C99.
>
> Conclusion: _socket_handle_client_data() does not reliably examine
> or report the right value of errno.
>
> One solution is to eliminate the messages issued from clr_fl(), and
> ensure that the function does not otherwise call any library functions
> before returning.
>
> A more general solution (because it doesn't impose any restrictions
> on the use of library functions) is to capture the value of errno
> immediately after an unsuccessful call to fcntl(), call fprintf(), and
> then return the captured value of errno.  If the call is successful,
> return zero.  For example:
>
> int clr_fl( int fd, int flags ) {
>
>         int val;
>
>         if( (val = fcntl( fd, F_GETFL, 0) ) < 0 ) {
>                 int local_errno = errno;
>                 fprintf(stderr, "fcntl F_GETFL error" );
>                 return local_errno;
>         }
>
>         val &= ~flags;
>
>         if( fcntl( fd, F_SETFL, val ) < 0 ) {
>                 int local_errno = errno;
>                 fprintf( stderr, "fcntl F_SETFL error" );
>                 return local_errno;
>         }
>         return 0;
> }
>
> I have not submitted a patch, partly because I don't know what
> approach you'd like to take, and partly because a change to the
> interface of clr_fl() will require changes to the calling functions
> as well.
>
> Scott McKellar
> http://home.swbell.net/mck9/aargh/
>
>


-- 
Mike Rylander