alexdobin / STAR

RNA-seq aligner
MIT License
1.83k stars 503 forks source link

GCC diagnostics from htslib #1174

Open nemequ opened 3 years ago

nemequ commented 3 years ago

Some of these look legit, some may arguably be false positives.

I also see some assignments inside of assert() calls in the diagnostics (there may be more in the code), which is generally considered to be a bad practice since that code will just go away if someone building it defines NDEBUG.

These are just with -Wall on GCC 10.2; you can probably find a lot more with -Wextra, -fanalyzer, and the occasional runs with -fsanitize=address,undefined. Also, clang generally has better diagnostics than GCC these days, plus they have scan-build. If your code builds on MSVC /W4 and /analyze are excellent, and of course Coverity Scan is free for open source and fantastic.

cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o kfunc.o kfunc.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o knetfile.o knetfile.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o kstring.o kstring.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o bgzf.o bgzf.c
bgzf.c: In function ‘bgzf_write’:
bgzf.c:733:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
  733 |     if ( !fp->is_compressed )
      |     ^~
bgzf.c:736:2: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
  736 |  const uint8_t *input = (const uint8_t*)data;
      |  ^~~~~
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o faidx.o faidx.c
faidx.c: In function ‘fai_load’:
faidx.c:265:5: warning: this ‘else’ clause does not guard... [-Wmisleading-indentation]
  265 |     else
      |     ^~~~
faidx.c:268:2: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘else’
  268 |  if (fp == 0) {
      |  ^~
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o hfile.o hfile.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o hfile_net.o hfile_net.c
echo '#define HTS_VERSION "0.0.1"' > version.h
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o hts.o hts.c
hts.c: In function ‘hts_idx_init’:
hts.c:522:56: warning: overflow in conversion from ‘uint32_t’ {aka ‘unsigned int’} to ‘int’ changes value from ‘idx->z.last_bin = 4294967295’ to ‘-1’ [-Woverflow]
  522 |  idx->z.save_bin = idx->z.save_tid = idx->z.last_tid = idx->z.last_bin = 0xffffffffu;
      |                                                        ^~~
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o sam.o sam.c
sam.c: In function ‘bam_hdr_write’:
sam.c:130:2: warning: ‘strncpy’ output truncated before terminating nul copying 4 bytes from a string of the same length [-Wstringop-truncation]
  130 |  strncpy(buf, "BAM\1", 4);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o synced_bcf_reader.o synced_bcf_reader.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o vcf_sweep.o vcf_sweep.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o tbx.o tbx.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o vcf.o vcf.c
vcf.c: In function ‘vcf_format’:
vcf.c:1909:4: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
 1909 |    if ( !first ) kputc(';', s); first = 0;
      |    ^~
vcf.c:1909:33: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
 1909 |    if ( !first ) kputc(';', s); first = 0;
      |                                 ^~~~~
vcf.c:1948:21: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
 1948 |                     if (!first) kputc(':', s); first = 0;
      |                     ^~
vcf.c:1948:48: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
 1948 |                     if (!first) kputc(':', s); first = 0;
      |                                                ^~~~~
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o vcfutils.o vcfutils.c
In file included from htslib/vcf.h:12,
                 from htslib/vcfutils.h:8,
                 from vcfutils.c:1:
vcfutils.c: In function ‘bcf_remove_alleles’:
vcfutils.c:291:25: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
  291 |                 assert( n=nG_ori );
      |                         ^
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/cram_codecs.o cram/cram_codecs.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/cram_decode.o cram/cram_decode.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/cram_encode.o cram/cram_encode.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/cram_index.o cram/cram_index.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/cram_io.o cram/cram_io.c
cram/cram_io.c: In function ‘cram_populate_ref’:
cram/cram_io.c:1529:27: warning: ‘.tmp_’ directive writing 5 bytes into a region of size between 1 and 4096 [-Wformat-overflow=]
 1529 |      sprintf(path_tmp, "%s.tmp_%d", path, /*getpid(),*/ i);
      |                           ^~~~~
cram/cram_io.c:1529:24: note: directive argument in the range [0, 2147483647]
 1529 |      sprintf(path_tmp, "%s.tmp_%d", path, /*getpid(),*/ i);
      |                        ^~~~~~~~~~~
cram/cram_io.c:1529:6: note: ‘sprintf’ output between 7 and 4111 bytes into a destination of size 4096
 1529 |      sprintf(path_tmp, "%s.tmp_%d", path, /*getpid(),*/ i);
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘full_path’,
    inlined from ‘cram_write_SAM_hdr’ at cram/cram_io.c:2987:3:
cram/cram_io.c:2917:6: warning: ‘strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation]
 2917 |      strncpy(out, in, PATH_MAX);
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
cram/cram_io.c:2909:2: warning: ‘strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation]
 2909 |  strncpy(out, in, PATH_MAX);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~
cram/cram_io.c: In function ‘cram_dopen’:
cram/cram_io.c:3291:2: warning: ‘strncpy’ specified bound 20 equals destination size [-Wstringop-truncation]
 3291 |  strncpy(def.file_id, filename, 20);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/cram_samtools.o cram/cram_samtools.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/cram_stats.o cram/cram_stats.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/files.o cram/files.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/mFILE.o cram/mFILE.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/md5.o cram/md5.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/open_trace_file.o cram/open_trace_file.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/pooled_alloc.o cram/pooled_alloc.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/sam_header.o cram/sam_header.c
In file included from cram/sam_header.c:36:
cram/sam_header.c: In function ‘sam_hdr_add_lines’:
cram/sam_header.c:328:13: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
  328 |      assert(p->next = t);
      |             ^
cram/sam_header.c: In function ‘sam_hdr_vadd’:
cram/sam_header.c:449:9: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
  449 |  assert(p->next = t);
      |         ^
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/string_alloc.o cram/string_alloc.c
In function ‘string_ndup’,
    inlined from ‘string_dup’ at cram/string_alloc.c:141:12:
cram/string_alloc.c:149:5: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
  149 |     strncpy(str, instr, len);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
cram/string_alloc.c: In function ‘string_dup’:
cram/string_alloc.c:141:12: note: length computed here
  141 |     return string_ndup(a_str, instr, strlen(instr));
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/thread_pool.o cram/thread_pool.c
cc -g -Wall -O2  -I. -DSAMTOOLS=1 -c -o cram/vlen.o cram/vlen.c
cram/vlen.c: In function ‘vflen’:
cram/vlen.c:121:9: warning: variable ‘i’ set but not used [-Wunused-but-set-variable]
  121 |     int i;
      |         ^
alexdobin commented 3 years ago

Hi Evan,

all these warnings are coming from the clone of the htslib, so it's not actually my code. :) Granted, this is a very old version of htslib, and I am long overdue switching to the latest htslib releases.

Cheers Alex