Re: ISOLATED, can't grok: 21.2 Showstopper? BAD DATA LOSS on disk full

SJT> Note that num_written is locally declared ssize_t (int), and lstr-> out_buffer_ind is size_t (unsigned long int? the definition of SJT> size_t is buried in /usr/lib/gcc-lib/i386-linux/2.95.3/include/stddef.h SJT> in about 50 nested #ifdefs HATE GCC HATE HATE 'scuse me): Everyone knows size_t is the same as unsigned long on real systems. SJT> Lstream_flush_out (lstr=0x84c44f8) SJT> at /coda/Projects/XEmacs/21.2-HEAD/src/lstream.c:350 SJT> 350 if (num_written == 0) SJT> (gdb) print num_written SJT> $24 = -1 SJT> (gdb) print lstr->out_buffer_ind SJT> $25 = 63 SJT> (gdb) step SJT> 356 else if (num_written >= lstr->out_buffer_ind) SJT> (gdb) SJT> 357 lstr->out_buffer_ind = 0; SJT> (gdb) print (num_written >= lstr->out_buffer_ind) SJT> $26 = 1 SJT> (gdb) print (num_written >= (int) lstr->out_buffer_ind) SJT> $27 = 0 SJT> (gdb) SJT> Aaaaarggggh! SJT> The following patch fixes this particular problem, but which of the SJT> other size_t members should be ssize_t?. (The funky -U 14 is to catch SJT> the whole struct.) What other bad things have happened? SJT> Index: src/lstream.h SJT> =================================================================== SJT> RCS file: /usr/CVSroot/XEmacs/xemacs/src/lstream.h,v SJT> retrieving revision 1.5.2.9 SJT> diff -u -U14 -r1.5.2.9 lstream.h SJT> --- lstream.h 2001/01/23 04:42:05 1.5.2.9 SJT> +++ lstream.h 2001/02/15 12:06:51 SJT> ＠＠ -145,29 +145,30 ＠＠ SJT> struct lstream SJT> { SJT> struct lcrecord_header header; SJT> const Lstream_implementation *imp; /* methods for this stream */ SJT> Lstream_buffering buffering; /* type of buffering in use */ SJT> size_t buffering_size; /* number of bytes buffered */ SJT> unsigned char *in_buffer; /* holds characters read from stream end */ SJT> size_t in_buffer_size; /* allocated size of buffer */ SJT> size_t in_buffer_current; /* number of characters in buffer */ SJT> size_t in_buffer_ind; /* pointer to next character to take from buffer */ SJT> unsigned char *out_buffer; /* holds characters to write to stream end */ SJT> size_t out_buffer_size; /* allocated size of buffer */ SJT> - size_t out_buffer_ind; /* pointer to next buffer spot to write a character */ SJT> + ssize_t out_buffer_ind; /* pointer to next buffer spot to write a character SJT> + gets compared to error returns, must be signed */ This is not correct. out_buffer_ind should never be negative. SJT> /* The unget buffer is more or less a stack -- things get pushed SJT> onto the end and read back from the end. Lstream_read() SJT> basically reads backwards from the end to get stuff; Lstream_unread() SJT> similarly has to push the data on backwards. */ SJT> unsigned char *unget_buffer; /* holds characters pushed back onto input */ SJT> size_t unget_buffer_size; /* allocated size of buffer */ SJT> size_t unget_buffer_ind; /* pointer to next buffer spot to write a character */ SJT> size_t byte_count; SJT> int flags; SJT> max_align_t data[1]; SJT> }; The following patch is completely untested, but it seems to me to have more correct logic. Stephen, please test this. Index: src/lstream.c =================================================================== RCS file: /usr/CVSroot/XEmacs/xemacs/src/lstream.c,v retrieving revision 1.12.2.17 diff -u -w -r1.12.2.17 lstream.c --- src/lstream.c 2001/01/24 06:41:45 1.12.2.17 +++ src/lstream.c 2001/02/16 14:12:22 ＠＠ -353,17 +353,17 ＠＠ the attempt to write the data might have resulted in an EWOULDBLOCK error. */ return 0; + else if (num_written < 0) + /* If error, just hold the data, for similar reasons as above. */ + return -1; else if (num_written >= lstr->out_buffer_ind) lstr->out_buffer_ind = 0; - else if (num_written > 0) + else { memmove (lstr->out_buffer, lstr->out_buffer + num_written, lstr->out_buffer_ind - num_written); lstr->out_buffer_ind -= num_written; } - else - /* If error, just hold the data, for similar reasons as above. */ - return -1; } if (lstr->imp->flusher)