Re: csplit 'write error' missing errno?

[Jim Meyering]

On Thu, Oct 22, 2015 at 12:22 PM, Assaf Gordon <address@hidden> wrote:
> Hello,
>
> In a long-running pipeline, we've encountered the following error message 
> with 'csplit':
>   csplit: write error for ‘[FILENAME]`
>
> It is very likely stemming from out-of-disk-space situation, but because 
> there was no errno printed, it's hard to know for sure, making 
> troubleshooting a bit frustrating.
>
> The error is printed in 'close_output_file()', in response to checking 
> 'ferror()'.
>
> I wonder if it would be beneficial to add the errno information to the error 
> message.
>
> >From a cursory look it seems 'close_output_file()' is (almost?) always 
> >called after
> 'dump_rest_of_file()' or 'save_line_to_file()' which themselves use 
> 'fwrite()'.
>
> So it could perhaps be assumed that if if 'ferror()' indicates an error on 
> the output stream,
> then the last operation was fwrite?
> The downside is that if my assumption doesn't hold, it could print a 
> misleading errno information.
>
> The following patch is a suggestion (not fully tested):
>
> ===
> diff --git a/src/csplit.c b/src/csplit.c
> index d966df5..d2a0228 100644
> --- a/src/csplit.c
> +++ b/src/csplit.c
> @@ -1004,7 +1004,7 @@ close_output_file (void)
>      {
>        if (ferror (output_stream))
>          {
> -          error (0, 0, _("write error for %s"), quote (output_filename));
> +          error (0, errno, _("write error for %s"), quote (output_filename));
>            output_stream = NULL;
>            cleanup_fatal ();
>          }
> ===

Hi Assaf,

Thanks for investigating.

That's a fundamental limitation of streams.
When detecting that error via ferror, the errno value is not
guaranteed to be useful.  From what I recall of the fwrite
specification, even immediately after a failed fwrite, errno is not
guaranteed to be useful, but in practice, it always is,
so coreutils programs do rely on that.

> I'll be able to troubleshoot and provide more information in couple of days.
> A more elaborate solution is to save the errno after 'fwrite()' in a variable,
> and print that variable. I can send such a patch if that's a better idea.

Yes, this sounds better.