Was: named pipe is borken and proposed fix, generally stdio maintenance

Peter Mazinger ps.m at gmx.net
Thu Jun 2 21:26:57 UTC 2011


Hi,

we have one big problem, the one who invented uClibc's stdio is not around, noone seems to want to touch this part. Personally I would replace it, but providing an alternative implementation as a config option would mean complete incompatibility that would be unmaintainable.
I know for ex. that the locking needed by some specific functions in stdio were applied generally to the whole source in uClibc_mutex.h. This added a lot of overhead and since than (0.9.29*) noone cared about it.

Peter
-------- Original-Nachricht --------
> Datum: Wed, 1 Jun 2011 14:49:14 -0700
> Von: "Jian Peng" <jipeng at broadcom.com>
> An: "uclibc at uclibc.org" <uclibc at uclibc.org>
> Betreff: named pipe is borken and proposed fix

> Testing procedure
> =================
> 
> Using following named pipe testing program, the testing process is as
> follow
> 
> In console A, run pipetest-server
> 
> $ ./pipetest-server
> pipetest-server got req (req from CL-A)
> pipetest-server got req (req from CL-B)
> 
> In console B, run pipetest-client
> 
> $ ./pipetest-client CL-A 20000000
> pipetest-client: CL-A: got rsp ack rsp
> 
> waiting for 20000000 microseconds...
> ending...
> 
> $ ./pipetest-client CL-B 20000000
> pipetest-client: CL-B: got rsp ack rsp
> 
> waiting for 20000000 microseconds...
> ending...
> 
> The above is the expected result.
> 
> But in uClibc, after "./pipetest-client CL-B 20000000", It hung, and there
> is no " pipetest-server got req (req from CL-B)" on pipetest-server.
> 
> This works on x86_64 host using glibc, and also works on MIPS using
> glibc-1.8.2, but it failed on MIPS using uClibc-0.9.32-rc3.
> 
> Testing programs pipetest-server.c
> ==================================
> 
> Here is testing program pipetest-server.c
> 
> #include <stdlib.h>
> #include <stdio.h>
> #include <signal.h>
> #include <unistd.h>
> #include <sys/stat.h>
> #include <sys/select.h>
> 
> #define REQNAME "/tmp/pipetest_req"
> #define RSPNAME "/tmp/pipetest_rsp"
> 
> int main( int argc, char **argv)
> {
>    int rc = 0;
>    FILE *pReq= 0;
>    FILE *pRsp= 0;
>    char buff[128];
> 
>    // Remove any existing file
>    remove( REQNAME );
>    remove( RSPNAME );
> 
>    rc= mknod( REQNAME, S_IFIFO|0666, 0);
>    if ( rc ) goto exit;
> 
>    rc= mknod( RSPNAME, S_IFIFO|0666, 0);
>    if ( rc ) goto exit;
> 
>    pReq= fopen( REQNAME, "r" );
>    if ( pReq )
>    {
>       fd_set rfds;
>       int fd= fileno( pReq );
> 
>       pRsp= fopen( RSPNAME, "w" );
>       for( ; ; )
>       {
>          FD_ZERO(&rfds);
>          FD_SET(fd, &rfds);
>          rc= select( fd+1, &rfds, 0, 0, 0);
>          
>          // Read next request
>          if ( fgets( buff, sizeof(buff), pReq ) != NULL )
>          {
>             printf( "pipetest-server got req (%s)\n", buff );
>             
>             fprintf( pRsp, "ack rsp\n" );
>             fflush( pRsp );            
>          }
>       }
>    }
> 
> exit:
> 
>    return rc;
> 
> }
> 
> Testing programs pipetest-client.c
> ==================================
> 
> Here is testing program pipetest-client.c
> 
> #include <stdlib.h>
> #include <stdio.h>
> #include <signal.h>
> #include <unistd.h>
> 
> #define REQNAME "/tmp/pipetest_req"
> #define RSPNAME "/tmp/pipetest_rsp"
> 
> int readIPCResponse( FILE *file, char *buff, int buffSize )
> {
>    int retryCount= 0;
> 
>    while( retryCount < 10 )
>    {
>       if ( fgets( buff, buffSize, file ) != NULL )
>       {
>          return 1;
>       }
>       
>       usleep( 5000 );
> 
>       ++retryCount;
>    }
> 
>    return 0;
> }
> 
> int main( int argc, char **argv)
> {
>    int rc = 0;
>    FILE *pReq= 0;
>    FILE *pRsp= 0;
>    int timeout= 10000000;
>    char *arg= 0;
>    char buff[128];
> 
>    arg = ( argc < 2 ) ? "ClientA" : argv[1];
>    timeout= ( argc < 3 ) ? 10000000 : atoi(argv[2]);
> 
>    pReq= fopen( REQNAME, "w" );
>    if ( pReq )
>    {
>       pRsp= fopen( RSPNAME, "r" );
>       if ( pRsp )
>       {
>          fprintf( pReq, "req from %s\n", arg );
>          fflush( pReq );
>          
>          rc= readIPCResponse( pRsp, buff, sizeof(buff) );
>          if ( rc )
>          {
>             printf( "pipetest-client: %s: got rsp %s\n", arg, buff );
>          }         
>             
>             
>          printf( "waiting for %d microseconds...\n", timeout );
>          usleep( timeout );
>          printf( "ending...\n" );
>             
>          fclose( pRsp );
>       }
>       fclose( pReq );
>    }
> 
>    return rc;
> 
> }
> 
> Debugging result
> =================
> 
> My debugging showed that in pipetest-server.c, after "./pipetest-client
> CL-A 20000000" finished, fgets() return NULL forever.
> 
> The problem is that after "./pipetest-client CL-A 20000000" finished, in
> stream associated with FIFO "/tmp/pipetest_req", fgets() will call
> __stdio_READ() internally
> 
>  24 size_t attribute_hidden __stdio_READ(register FILE *stream,
>  25                                         unsigned char *buf, size_t
> bufsize)
>  26 {
>  27         ssize_t rv = 0;
>  28
> 
>  36         if (!__FEOF_UNLOCKED(stream)) {
>  37                 if (bufsize > SSIZE_MAX) {
>  38                         bufsize = SSIZE_MAX;
>  39                 }
>  40
>  41 #ifdef __UCLIBC_MJN3_ONLY__
>  42 #warning EINTR?
>  43 #endif
>  44 /*      RETRY: */
>  45                 if ((rv = __READ(stream, (char *) buf, bufsize)) <= 0)
> {
>  46                         if (rv == 0) {
>  47                                 __STDIO_STREAM_SET_EOF(stream);
> 
> At line 45, rv return 0, then at line 47, it set __FLAG_EOF in
> stream->__modeflags.
> 
> After "./pipetest-client CL-B 20000000" was started, it writed " req from
> CL-B" to FIFO "/tmp/pipetest_req", 
> but since __FLAG_EOF was set in stream->__modeflags in above stream, it
> failed at line 36, and no longer call __READ() at line 45 to read new message
> from FIFO.
> 
> The problem is that FIFO is a special file type, and the conventional EOF
> is useless, and should not be set.
> 
> My proposed fix is to avoid setting __FLAG_EOF for stream associated with
> FIFO after sender was closed.
> 
> My patch 
> ========
> 
> This is my patch to fix it. Sorry that git server of uClibc.org is down
> today, so I cannot generate diff using git.
> 
> The patch worked well in my testing case.
> 
> In order to call fstat(fd, stat) inside libc, I have to copy and paste
> fstat.c into _READ.c, it is not nice, but I want to get your input on whether
> this is the right direction or not at this moment.
> 
> Thanks,
> 
> Jian
> 
> 
> diff -Naur -p uClibc-old/libc/stdio/_READ.c uClibc-new/libc/stdio/_READ.c
> --- uClibc-old/libc/stdio/_READ.c	2011-06-01 14:10:29.904699508 -0700
> +++ uClibc-new/libc/stdio/_READ.c	2011-06-01 14:17:15.057396809 -0700
> @@ -6,7 +6,25 @@
>   */
>  
>  #include "_stdio.h"
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +#include <sys/stat.h>
> +#include "xstatconv.h"
>  
> +#define __NR___syscall_fstat __NR_fstat
> +static __inline__ _syscall2(int, __syscall_fstat, int, fd, struct
> kernel_stat *, buf)
> +
> +int _my_fstat(int fd, struct stat *buf)
> +{
> +	int result;
> +	struct kernel_stat kbuf;
> +
> +	result = __syscall_fstat(fd, &kbuf);
> +	if (result == 0) {
> +		__xstat_conv(&kbuf, buf);
> +	}
> +	return result;
> +}
>  
>  /* Given a reading stream without its end-of-file indicator set and
>   * with no buffered input or ungots, read at most 'bufsize' bytes
> @@ -44,7 +62,16 @@ size_t attribute_hidden __stdio_READ(reg
>  /* 	RETRY: */
>  		if ((rv = __READ(stream, (char *) buf, bufsize)) <= 0) {
>  			if (rv == 0) {
> -				__STDIO_STREAM_SET_EOF(stream);
> +				struct stat stat;
> +
> +				if( (stream->__modeflags & __FLAG_FIFOFILE) ||
> +					_my_fstat(stream->__filedes, &stat) >= 0) {
> +					if(S_ISFIFO(stat.st_mode))
> +						stream->__modeflags |= __FLAG_FIFOFILE;
> +				}
> +
> +				if(!(stream->__modeflags & __FLAG_FIFOFILE))
> +					__STDIO_STREAM_SET_EOF(stream);
>  			} else {
>  /* 				if (errno == EINTR) goto RETRY; */
>  				__STDIO_STREAM_SET_ERROR(stream);
> diff -Naur -p uClibc-old/libc/sysdeps/linux/common/bits/uClibc_stdio.h
> uClibc-new/libc/sysdeps/linux/common/bits/uClibc_stdio.h
> --- uClibc-old/libc/sysdeps/linux/common/bits/uClibc_stdio.h	2011-06-01
> 14:09:55.974786150 -0700
> +++ uClibc-new/libc/sysdeps/linux/common/bits/uClibc_stdio.h	2011-06-01
> 14:17:13.216060881 -0700
> @@ -331,6 +331,7 @@ struct __STDIO_FILE_STRUCT {
>  #define __FLAG_FREEBUF		0x4000U
>  #define __FLAG_LARGEFILE	0x8000U /* fixed! == 0_LARGEFILE for linux */
>  #define __FLAG_FAILED_FREOPEN	__FLAG_LARGEFILE
> +#define __FLAG_FIFOFILE		0x1000U /* handle FIFO differently */
>  
>  /* Note: In no-buffer mode, it would be possible to pack the necessary
>   * flags into one byte.  Since we wouldn't be buffering and there would
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> _______________________________________________
> uClibc mailing list
> uClibc at uclibc.org
> http://lists.busybox.net/mailman/listinfo/uclibc

-- 
Empfehlen Sie GMX DSL Ihren Freunden und Bekannten und wir
belohnen Sie mit bis zu 50,- Euro! https://freundschaftswerbung.gmx.de


More information about the uClibc mailing list