bugThe nmh Mail Handling System - Bugs: bug #50925, Lack of robustness when disk is...

 
 

bug #50925: Lack of robustness when disk is full

Submitted by:  Ralph Corderoy <ralph>
Submitted on:  Wed 03 May 2017 09:05:45 AM UTC  
 
Category:  Bug Severity:  3 - Normal
Priority:  5 - Normal Status:  None
Assigned to:  None Open/Closed:  Open

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Mon 29 Oct 2018 04:54:09 PM UTC, comment #11:

fflush after every fputs, etc., seems a bad idea as it discards the
buffering benefit of stdio in avoiding write(2) and its syscall
overhead. But checking fputs's return value, and all the other fprintf,
etc, is fine as it can still return `disk full', along with other
errors, and save effort by breaking the loop.

stdio errors are sticky. If fputs("a\n") write(2)s and gets `disk
full', and then fputs("b\n") write(2)s and doesn't, the FILE still has
its error indicator set. So the error isn't lost by checking fclose in
done().

An fclose(stdout) would return any error without having to fflush() and
ferror(). stderr would still need the fflush() and ferror() because
otherwise file descriptor 2 would be close(2)d by the fclose().

freopen(3) does discard any existing error state, so a call to it is a
contender for fflush+ferror before the freopen.

As for dying in the middle of an inc(1) due to the scan having trouble,
that already happens thanks to SIGPIPE, e.g. `inc | sed 42q', or more
likely `inc | less' and not reading all the input. And I think that's
fine. (Perhaps the inc should happen silently, and then a scan follow.)

Ralph Corderoy <ralph>
Project Member
Mon 29 Oct 2018 11:59:44 AM UTC, comment #10:

Would it make sense to call fflush() and ferror() on both stdout and sterr in done()? That could provide global handling of errors with minimum code change.

There are a few uses of freopen(), in push.c and slocal.c. I didn't look to see if they could mask an earlier error. If so, maybe add the calls before calling it?

David Levine <levine>
Project Member
Sun 28 Oct 2018 11:52:34 PM UTC, comment #9:

I'm OK with calling them at the end. Maybe we'd miss an odd error, but it's better than what we have now (where now is before my last commit).

David Levine <levine>
Project Member
Sun 28 Oct 2018 09:41:25 PM UTC, comment #8:

Maybe what makes sense is at the end of every program we call fflush(stdout) and/or ferror(stdout), and return an error if stdout has an error set?

I am uncomfortable modifying scansbr.c, as that's used by inc(1); calling adios() in the middle of that could cause bad things to happen.

Ken Hornstein <kenh>
Project Administrator
Sun 28 Oct 2018 06:45:08 PM UTC, comment #7:

I added a fflush() after each fputs() to be sure to not lose the error. And that would be easy to turn into a macro. But, not the best approach in terms of performance. Any other ideas?

David Levine <levine>
Project Member
Sun 28 Oct 2018 06:30:29 PM UTC, comment #6:

commit f54674f1b5d07148d9655fc3b337ad5f725272a7
scan(1) now checks for full output device [Bug #50925].

Added fflush(3) after every output line: may slow down scan
on output with large number of lines.

David Levine <levine>
Project Member
Sun 28 Oct 2018 04:53:16 PM UTC, comment #5:

Oh, I think commit a scan fix now. No point waiting for some day when
it's all going to be fixed in step. This bug was mainly about using
/dev/full in our tests so we shake some of these problems out and tackle
the important ones; those, like scan, where a user might think they've
got the useful data saved and delete the email.

Ralph Corderoy <ralph>
Project Member
Sun 28 Oct 2018 01:58:44 PM UTC, comment #4:

Indeed, fflush() reports the error. There are many (73) unchecked calls to fflush() of stdout or stderr. I didn't look to see if there are calls to fputs() on stdout or stderr without a subsequent fflush(). I don't have time now to go after all that.

Should I commit the fix to scan now, or wait for all of this to be addressed at once?

David Levine <levine>
Project Member
Sat 27 Oct 2018 09:51:20 PM UTC, comment #3:

fputs(3) is stdio.h so uses buffering. When a write(2) is attempted, either
during or after the fputs(), the full disk will be seen. A pending write()
can be forced with fflush(3) or fclose(3), amongst others. A `sticky' error
from an earlier action can be tested for with ferror(3), though errno might
have been trampled by something else by then.

stdout to a TTY would be line buffered. I'm guessing it's not with /dev/full
so, with the test program, the write() isn't happening before fputs() returns
that all is well.

Ralph Corderoy <ralph>
Project Member
Sat 27 Oct 2018 03:57:43 PM UTC, comment #2:

scan uses fputs() and doesn't check its return value. But even if it did, it doesn't seem to report ENOSPC (on Linux 4,18.8), as shown by attached program:
$ ./fputs_or_write >/dev/full
fputs() returned 1
$ ./fputs_or_write 1 >/dev/full
write() returned -1

Not sure where that leaves us.

(file #45312)

David Levine <levine>
Project Member
Wed 03 May 2017 09:10:17 AM UTC, comment #1:

$ date >/dev/full
/bin/date: write error: No space left on device
$ echo $?
1
$ scan . >/dev/full
$ echo $?
0

packf(1) does check. Others may or may not. Could do with using /dev/full, if it's available, for more make-check tests.

Ralph Corderoy <ralph>
Project Member
Wed 03 May 2017 09:05:45 AM UTC, original submission:
Ralph Corderoy <ralph>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #45312:  fputs_or_write.c added by levine (453B - text/x-csrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by kenh (Posted a comment)
  • -email is unavailable- added by levine (Updated the item)
  • -email is unavailable- added by ralph (Submitted the item)
  •  

     

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-10-27 levine Attached File- => Added fputs_or_write.c, #45312

    Back to the top


    Powered by Savane 3.3