Reduce use of abort, remove assert(fp), pass one string to fprintf no…

…t multicall Reduce use of abort to situations where you might actually want to debug. Remove assert(fp)'s and replace with proper error messages. Remove a few instances where fprintf is called multiple times rather than just passing one string to it. Signed-off-by: Martin O. Pollard <mp15@sanger.ac.uk>
samtools · Aug 28, 2014 · 7b5a9ea · 7b5a9ea
1 parent 57e71e7
commit 7b5a9ea
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 30 deletions.
diff --git a/bam.c b/bam.c
@@ -26,7 +26,6 @@ DEALINGS IN THE SOFTWARE.  */
 #include <stdio.h>
 #include <ctype.h>
 #include <errno.h>
-#include <assert.h>
 #include "bam.h"
 #include "htslib/kstring.h"
 #include "sam_header.h"

diff --git a/bam_md.c b/bam_md.c
@@ -24,7 +24,6 @@ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 DEALINGS IN THE SOFTWARE.  */
 
 #include <unistd.h>
-#include <assert.h>
 #include <string.h>
 #include <ctype.h>
 #include <math.h>

diff --git a/bam_plbuf.c b/bam_plbuf.c
@@ -25,7 +25,6 @@ DEALINGS IN THE SOFTWARE.  */
 #include <stdio.h>
 #include <stdlib.h>
 #include <ctype.h>
-#include <assert.h>
 #include <htslib/hts.h>
 #include <htslib/sam.h>
 #include "bam_plbuf.h"

diff --git a/bam_sort.c b/bam_sort.c
@@ -26,7 +26,6 @@ DEALINGS IN THE SOFTWARE.  */
 #include <stdbool.h>
 #include <stdlib.h>
 #include <ctype.h>
-#include <assert.h>
 #include <errno.h>
 #include <stdio.h>
 #include <string.h>

diff --git a/bam_split.c b/bam_split.c
@@ -28,7 +28,6 @@ DEALINGS IN THE SOFTWARE.  */
 #include <stdlib.h>
 #include <stdbool.h>
 #include <limits.h>
-#include <assert.h>
 #include <unistd.h>
 #include <regex.h>
 #include <htslib/khash.h>

diff --git a/bam_stat.c b/bam_stat.c
@@ -23,8 +23,8 @@ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 DEALINGS IN THE SOFTWARE.  */
 
 #include <unistd.h>
-#include <assert.h>
 #include "bam.h"
+#include "samtools.h"
 
 typedef struct {
     long long n_reads[2], n_mapped[2], n_pair_all[2], n_pair_map[2], n_pair_good[2];
@@ -84,8 +84,11 @@ int bam_flagstat(int argc, char *argv[])
         fprintf(stderr, "Usage: samtools flagstat <in.bam>\n");
         return 1;
     }
-    fp = strcmp(argv[optind], "-")? bam_open(argv[optind], "r") : bam_dopen(fileno(stdin), "r");
-    assert(fp);
+    fp = strcmp(argv[optind], "-")? bam_open(argv[optind], "r") : bam_dopen(STDIN_FILENO, "r");
+    if (fp == NULL) {
+        print_error_errno("Cannot open input file \"%s\"", argv[optind]);
+        return 1;
+    }
     header = bam_header_read(fp);
     s = bam_flagstat_core(fp);
     printf("%lld + %lld in total (QC-passed reads + QC-failed reads)\n", s->n_reads[0], s->n_reads[1]);

diff --git a/bam_tview.c b/bam_tview.c
@@ -317,7 +317,7 @@ int base_draw_aln(tview_t *tv, int tid, int pos)
         if ( !tv->ref )
         {
             fprintf(stderr,"Could not read the reference sequence. Is it seekable (plain text or compressed + .gzi indexed with bgzip)?\n");
-            abort();
+            exit(1);
         }
     }
     // draw aln
@@ -341,13 +341,12 @@ static void error(const char *format, ...)
 {
     if ( !format )
     {
-        fprintf(stderr, "\n");
-        fprintf(stderr, "Usage: samtools tview [options] <aln.bam> [ref.fasta]\n");
-        fprintf(stderr, "Options:\n");
-        fprintf(stderr, "   -d display      output as (H)tml or (C)urses or (T)ext \n");
-        fprintf(stderr, "   -p chr:pos      go directly to this position\n");
-        fprintf(stderr, "   -s STR          display only reads from this sample or group\n");
-        fprintf(stderr, "\n");
+        fprintf(stderr,
+"Usage: samtools tview [options] <aln.bam> [ref.fasta]\n"
+"Options:\n"
+"   -d display      output as (H)tml or (C)urses or (T)ext \n"
+"   -p chr:pos      go directly to this position\n"
+"   -s STR          display only reads from this sample or group\n");
     }
     else
     {
@@ -408,7 +407,7 @@ int bam_tview_main(int argc, char *argv[])
             break;
             }
     }
-    if(tv==NULL)
+    if (tv==NULL)
     {
         error("cannot create view");
         return EXIT_FAILURE;

diff --git a/bamshuf.c b/bamshuf.c
@@ -31,6 +31,7 @@ DEALINGS IN THE SOFTWARE.  */
 #include "htslib/sam.h"
 #include "htslib/bgzf.h"
 #include "htslib/ksort.h"
+#include "samtools.h"
 
 #define DEF_CLEVEL 1
 
@@ -72,7 +73,7 @@ static inline int elem_lt(elem_t x, elem_t y)
 
 KSORT_INIT(bamshuf, elem_t, elem_lt)
 
-static void bamshuf(const char *fn, int n_files, const char *pre, int clevel, int is_stdout)
+static int bamshuf(const char *fn, int n_files, const char *pre, int clevel, int is_stdout)
 {
     BGZF *fp, *fpw, **fpt;
     char **fnt, modew[8];
@@ -83,7 +84,11 @@ static void bamshuf(const char *fn, int n_files, const char *pre, int clevel, in
 
     // split
     fp = strcmp(fn, "-")? bgzf_open(fn, "r") : bgzf_dopen(fileno(stdin), "r");
-    assert(fp);
+    if (fp == NULL) {
+        print_error_errno("Cannot open input file \"%s\"", fn);
+        return 1;
+    }
+
     h = bam_hdr_read(fp);
     fnt = (char**)calloc(n_files, sizeof(char*));
     fpt = (BGZF**)calloc(n_files, sizeof(BGZF*));
@@ -93,6 +98,10 @@ static void bamshuf(const char *fn, int n_files, const char *pre, int clevel, in
         fnt[i] = (char*)calloc(l + 10, 1);
         sprintf(fnt[i], "%s.%.4d.bam", pre, i);
         fpt[i] = bgzf_open(fnt[i], "w1");
+        if (fpt[i] == NULL) {
+            print_error_errno("Cannot open intermediate file \"%s\"", fnt[i]);
+            return 1;
+        }
         bam_hdr_write(fpt[i], h);
     }
     b = bam_init1();
@@ -114,6 +123,15 @@ static void bamshuf(const char *fn, int n_files, const char *pre, int clevel, in
         fpw = bgzf_open(fnw, modew);
         free(fnw);
     } else fpw = bgzf_dopen(fileno(stdout), modew); // output to stdout
+    if (fpw == NULL) {
+	if (is_stdout) {
+            print_error_errno("Cannot open stdout for writing");
+        } else {
+            print_error_errno("Cannot open output file \"%s.bam\"", pre);
+        }
+        return 1;
+    }
+
     bam_hdr_write(fpw, h);
     bam_hdr_destroy(h);
     for (i = 0; i < n_files; ++i) {
@@ -139,6 +157,7 @@ static void bamshuf(const char *fn, int n_files, const char *pre, int clevel, in
     }
     bgzf_close(fpw);
     free(fnt); free(cnt);
+    return 0;
 }
 
 int main_bamshuf(int argc, char *argv[])
@@ -154,14 +173,14 @@ int main_bamshuf(int argc, char *argv[])
     }
     if (is_un) clevel = 0;
     if (optind + 2 > argc) {
-        fprintf(stderr, "\nUsage:   samtools bamshuf [-Ou] [-n nFiles] [-c cLevel] <in.bam> <out.prefix>\n\n");
-        fprintf(stderr, "Options: -O      output to stdout\n");
-        fprintf(stderr, "         -u      uncompressed BAM output\n");
-        fprintf(stderr, "         -l INT  compression level [%d]\n", DEF_CLEVEL);
-        fprintf(stderr, "         -n INT  number of temporary files [%d]\n", n_files);
-        fprintf(stderr, "\n");
+        fprintf(stderr,
+"Usage:   samtools bamshuf [-Ou] [-n nFiles] [-c cLevel] <in.bam> <out.prefix>\n\n"
+"Options: -O      output to stdout\n"
+"         -u      uncompressed BAM output\n"
+"         -l INT  compression level [%d]\n" // DEF_CLEVEL
+"         -n INT  number of temporary files [%d]\n", // n_files
+                DEF_CLEVEL, n_files);
         return 1;
     }
-    bamshuf(argv[optind], n_files, argv[optind+1], clevel, is_stdout);
-    return 0;
+    return bamshuf(argv[optind], n_files, argv[optind+1], clevel, is_stdout);
 }
diff --git a/bamtk.c b/bamtk.c
@@ -24,7 +24,6 @@ DEALINGS IN THE SOFTWARE.  */
 
 #include <stdio.h>
 #include <unistd.h>
-#include <assert.h>
 #include <fcntl.h>
 #include <stdarg.h>
 #include <string.h>

diff --git a/padding.c b/padding.c
@@ -89,9 +89,9 @@ static void unpad_seq(bam1_t *b, kstring_t *s)
             /* do nothing */
         } else if (op == BAM_CDEL) {
             for (i = 0; i < ol; ++i) s->s[s->l++] = 0;
-                } else {
+        } else {
             fprintf(stderr, "[depad] ERROR: Didn't expect CIGAR op %c in read %s\n", BAM_CIGAR_STR[op], bam1_qname(b));
-                        assert(-1);
+            exit(1);
         }
     }
     assert(length == s->l);