Re: [Bio-bwa-help] File writing in BWA does not have error checking

[Alec W. <al...@br...>]

Hi John,

Thanks for your comments.  I've made all the changes you suggest.  I 
added err_fclose(stdout) at the end of main().  I will run some tests to 
make sure I haven't broken anything and then circulate a patch.

-Alec

On 4/21/11 12:11 PM, John Marshall wrote:
> On 19 Apr 2011, at 19:28, Alec Wysoker wrote:
>> I have patched the 0.5.9 tag in order to fix this problem.  Attached 
>> are two new source files, and the patch file for the existing sources.
>
> Me, I certainly agree with adding this error checking.  We've been 
> bitten by such .sai truncation in the past, and it was not fun to 
> diagnose the problem.  It's possible that our problem was the delayed 
> not-reported-to-the-process kind mentioned by Mark; we don't really 
> know and I'd agree that that's relatively rare.  In any case, the 
> existence of rare unreported errors is no reason not to fail on the 
> ones that are detectable!
>
> Some comments on the patch:
>
> 1. Perhaps these functions could be added to utils.[ch], which 
> contains similar do-stdio-or-die sorts of functions, rather than 
> adding more small source files.
>
> 2. You should also add a safe_fclose() (or maybe xclose() like the 
> existing functions in utils.h... hmmm... xwrite() etc?) and use 
> safe_fclose(stdout) at the end of writing the files.  This will catch 
> errors that even a final fflush(stdout) would not -- see the close(2) 
> man page on linux.  (A final fflush(stdout) just before closing it 
> would then be redundant, of course.)
>
> 3.     fprintf(stderr, "fwrite failed with error %d\n", ferror(stream));
>
> ferror() just returns true or false; what you want to print out is 
> errno.  Probably best via strerror(), something like this:
>
>     fprintf(stderr, "can't write output: %s\n", strerror(errno));
>
> For the safe_printf() one, do this (or at least make a copy of errno) 
> before the va_end() as just about anything in the standard library, 
> including va_end(), can in theory change errno before you look at it.
>
> 4. For future flexibility, it might be nice to provide safe_fprintf() 
> rather than safe_printf().
>
> Cheers,
>
>     John
>
>