Hi bug-gnulib, I'd like to reopen a discussion on this list from last September: <http://lists.gnu.org/archive/html/bug-gnulib/2004-09/msg00010.html>.
At the end of that discussion I didn't create the blocking-io module due mostly to Paul Eggert's objections (and a lack of time on my part, especially for a solution which sounded like it would be rejected). Anyhow, RMS wanted the problem this fixes in CVS fixed badly enough that he wrote most of the stdio wrappers we discussed himself. The public portion of that new discussion is archived here: <http://lists.gnu.org/archive/html/bug-cvs/2005-07/msg00017.html>. In any case, after a few iterations and testing in CVS on GNU/Linux, RMS & I came up with the two files I've attached to this email. I believe they answer all of the objections raised in the first thread linked above except Bruno's comment on porting to WOE32 and the following comments, which I will attempt to address here. The WOE32 issue and conversion to a proper GNULIB module can wait until the disposition of this code is settled. Paul Eggert wrote: >You have to include stdio.h first, then #undef fwrite, then >#define fwrite. It can be done (Emacs did it, after all), but I don't >recommend it. > Isn't this the standard behavior for a GNULIB module's header file? This is almost exactly what unlocked-io does, after all. In any case, I think this is now addressed by the functionality in blocking-io.h. >/ POSIX says fflush sets errno. I'm worried about implementations that/ >/ don't conform (or perhaps conform to an older standard?)./ In the cases where fflush does not set errno, we are no worse off than we were before. The way the new wrappers are written, the function behaviors should remain unchanged if errno != EAGAIN (or EWOULDBLOCK on BSD) and when when they do not return short writes. >As far as I can tell POSIX doesn't guarantee that repeated fflushes >will eventually write all the data, with no duplicates, in this >situation. Here's the spec: >http://www.opengroup.org/onlinepubs/009695399/functions/fflush.html > Maybe not, but isn't this by far the implication of EAGAIN in most other contexts? Couldn't do what you asked this time, try *AGAIN*? I can understand the problem with fwrite and an object size > 1 since there is no way for the function to return a partial item count, but once stdio has the data, why should fflush not track what was written properly? Aside from bugs, this is what I would expect from most fflush implementations. How about we install the module and see what sort of problem reports we get? Bruno Habile wrote: >Larry Jones wrote: >>/ they should be testing for writability with select before trying to/ >>/ write to stderr rather than just blindly putting stderr into nonblocking/ >>/ mode/ > >Agreed: On platforms where select() or poll() is available, there is no >reason to use the old non-blocking I/O facility. > With this fix, CVS and potentially other tools will work with the broken SSH implementations and other, hypothetical, future tools, which are as rude in their treatment of shared file descriptors, regardless of whether the OpenSSH folks fix this issue. I would like to see this installed in several locations for the following reasons, all basically various aspects of the advantages of code-sharing: GLIBC: 1. This module provides useful functionality often expected by programs: guaranteed blocking behavior from stdio. 2. Would promote use in other tools, avoiding similar problems in the future. 3. Would be ported to more platforms more quickly, decreasing liklihood of broken CVS releases. 4. More eyes on this code would mean fewer bugs. GNULIB: 1. All the reasons I want this in GLIBC. 2. Ease of importing GLIBC updates to this code into CVS. CVS: 1. This fix certainly shouldn't hurt anything, once the portability issues are dealt with. 2. Will hopefully fix interaction with OpenSSH, and potentially other tools, on most platforms. I might get away with installing this just in CVS, but I am afraid that CVS doesn't have the manpower to deal with the potential porting issues raised by various parties in the earlier discussions without the GLIBC and GNULIB support, and RMS said he would back getting this into GLIBC & GNULIB to get the fix into CVS. Regardless, as I already stated, I think this module provides useful functionality often expected by programs: guaranteed blocking behavior from stdio. Regards, Derek
#ifndef BLOCKING_IO_H # define BLOCKING_IO_H /* Get va_list. */ # include <stdarg.h> # include <stdio.h> # define printf blocking_printf # define fprintf blocking_fprintf # define vprintf blocking_vprintf # define vfprintf blocking_vfprintf # undef putchar # define putchar blocking_putchar # undef putc # define putc blocking_putc # define fputc blocking_putc # define puts blocking_puts # define fputs blocking_fputs # define fwrite blocking_fwrite # define fflush blocking_fflush # define fclose blocking_fclose int blocking_printf (const char *format, ...); int blocking_fprintf (FILE *stream, const char *format, ...); int blocking_vprintf (const char *format, va_list ap); int blocking_vfprintf (FILE *stream, const char *format, va_list ap); int blocking_putchar (char c); int blocking_putc (char c, FILE *stream); int blocking_puts (const char *s); int blocking_fputs (const char *s, FILE *stream); int blocking_fwrite (const void *buffer, size_t size, size_t count, FILE *stream); int blocking_fflush (FILE *stream); int blocking_fclose (FILE *stream); #endif /* BLOCKING_IO_H */
/* The idea of this module is to help programs cope with output streams that have been set nonblocking. To use it, simply make these definitions in your other files: #define printf blocking_printf #define fprintf blocking_fprintf #define vprintf blocking_vprintf #define vfprintf blocking_vfprintf #undef putchar #define putchar blocking_putchar #undef putc #define putc blocking_putc #define fputc blocking_putc #define puts blocking_puts #define fputs blocking_fputs #define fwrite blocking_fwrite #define fflush blocking_fflush and link with this module. */ #include <errno.h> #include <stdarg.h> #include <stddef.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <sys/select.h> /* EWOULDBLOCK is not defined by POSIX, but some BSD systems will return it, rather than EAGAIN, for nonblocking writes. */ #ifdef EWOULDBLOCK # define blocking_error(err) ((err) == EWOULDBLOCK || (err) == EAGAIN) #else # define blocking_error(err) ((err) == EAGAIN) #endif int blocking_fwrite (const void *buffer, size_t size, size_t count, FILE *stream) { size_t count_left = count; size_t count_written = 0; int desc = fileno (stream); if (size == 0 || count == 0) return 0; /* Handle the case where SIZE * COUNT overflows. (The comparison is designed to be conservative.) */ if (size > 1 && (double)size * (double)count > SIZE_MAX / 4) { const char *p = (const char *) buffer; for (count_written = 0; count_written < count; count_written++) { if (blocking_fwrite (p, sizeof *p, size, stream) < size) break; p += size; } return count_written; } /* When SIZE is > 1, convert to the byte-wise case since that is the only way fwrite undertakes not to repeat writing part of the data. */ if (size > 1) { size_t total = size * count; unsigned int value = blocking_fwrite (buffer, sizeof *buffer, total, stream); /* It is ok, according to POSIX, if VALUE is not a multiple of SIZE, because fwrite in that case can write part of an object. */ return value / size; } while (1) { int written; fd_set set; written = fwrite (buffer, sizeof *buffer, count_left, stream); count_written += written; if (written == count_left) break; if (blocking_error (errno)) break; /* Wait for space to be available. */ FD_ZERO (&set); FD_SET (desc, &set); select (desc + 1, NULL, &set, NULL, NULL); buffer += written; count_left -= written; } return count_written; } int blocking_printf (const char *format, ...) { int out_count; char *buffer; int size = 1000; while (1) { va_list argptr; buffer = malloc (size); if (!buffer) { /* Failed malloc on some non-posix systems (e.g. mingw) fail to set errno. */ if (!errno) errno = ENOMEM; return 0; } /* Format it into BUFFER, as much as fits. */ va_start (argptr, format); out_count = vsnprintf (buffer, size, format, argptr); va_end (argptr); /* OUT_COUNT now is number of bytes needed, not counting final null. */ if (out_count + 1 <= size) break; /* If it did not fit, try again with more space. */ free (buffer); size = out_count + 1; } /* Now we have the desired output in BUFFER. Output it. */ blocking_fwrite (buffer, sizeof *buffer, out_count, stdout); free (buffer); return out_count; } int blocking_fprintf (FILE *stream, const char *format, ...) { int out_count; char *buffer; int size = 1000; while (1) { va_list argptr; buffer = malloc (size); if (!buffer) { /* Failed malloc on some non-posix systems (e.g. mingw) fail to set errno. */ if (!errno) errno = ENOMEM; return 0; } /* Format it into BUFFER, as much as fits. */ va_start (argptr, format); out_count = vsnprintf (buffer, size, format, argptr); va_end (argptr); /* OUT_COUNT now is number of bytes needed, not counting final null. */ if (out_count + 1 <= size) break; /* If it did not fit, try again with more space. */ free (buffer); size = out_count + 1; } /* Now we have the desired output in BUFFER. Output it. */ blocking_fwrite (buffer, sizeof *buffer, out_count, stream); free (buffer); return out_count; } int blocking_vprintf (const char *format, va_list ap) { int out_count; char *buffer; int size = 1000; while (1) { va_list argptr; buffer = malloc (size); if (!buffer) { /* Failed malloc on some non-posix systems (e.g. mingw) fail to set errno. */ if (!errno) errno = ENOMEM; return 0; } /* Format it into BUFFER, as much as fits. */ va_copy (argptr, ap); out_count = vsnprintf (buffer, size, format, argptr); va_end (argptr); /* OUT_COUNT now is number of bytes needed, not counting final null. */ if (out_count + 1 <= size) break; /* If it did not fit, try again with more space. */ free (buffer); size = out_count + 1; } /* Now we have the desired output in BUFFER. Output it. */ blocking_fwrite (buffer, sizeof *buffer, out_count, stdout); free (buffer); return out_count; } int blocking_vfprintf (FILE *stream, const char *format, va_list ap) { int out_count; char *buffer; int size = 1000; while (1) { va_list argptr; buffer = malloc (size); if (!buffer) { /* Failed malloc on some non-posix systems (e.g. mingw) fail to set errno. */ if (!errno) errno = ENOMEM; return 0; } /* Format it into BUFFER, as much as fits. */ va_copy (argptr, ap); out_count = vsnprintf (buffer, size, format, argptr); va_end (argptr); /* OUT_COUNT now is number of bytes needed, not counting final null. */ if (out_count + 1 <= size) break; /* If it did not fit, try again with more space. */ free (buffer); size = out_count + 1; } /* Now we have the desired output in BUFFER. Output it. */ blocking_fwrite (buffer, sizeof *buffer, out_count, stream); free (buffer); return out_count; } int blocking_putchar (char c) { return (blocking_fwrite (&c, sizeof c, 1, stdout) < 1 ? EOF : (int) (unsigned char) c); } /* This serves for fputs also. */ int blocking_putc (char c, FILE *stream) { return (blocking_fwrite (&c, sizeof c, 1, stream) < 1 ? EOF : (int) (unsigned char) c); } int blocking_puts (const char *s) { size_t len = strlen (s); return (blocking_fwrite (s, sizeof *s, len, stdout) < len ? EOF : 0); } int blocking_fputs (const char *s, FILE *stream) { size_t len = strlen (s); return (blocking_fwrite (s, sizeof *s, len, stream) < len ? EOF : 0); } int blocking_fflush (FILE *stream) { int desc = fileno (stream); while (1) { int value; fd_set set; value = fflush (stream); if (value == 0 || blocking_error (errno)) return value; /* Wait for space to be available. */ FD_ZERO (&set); FD_SET (desc, &set); select (desc + 1, NULL, &set, NULL, NULL); } } int blocking_fclose (FILE *stream) { int desc = fileno (stream); while (1) { int value; fd_set set; value = fclose (stream); if (value == 0 || blocking_error (errno)) return value; /* Wait for space to be available. */ FD_ZERO (&set); FD_SET (desc, &set); select (desc + 1, NULL, &set, NULL, NULL); } }
_______________________________________________ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib