Add internal isdigit_c()/etc functions for plain chars

jmarshall · jmarshall · commit fc9aeb6f7766 · 2016-02-19T10:52:17.000Z
Using `char c; ... isdigit(c)` is technically incorrect (as the
&lt;ctype.h&gt; functions expect an int such as is returned by fgetc()) and
produces a warning on Windows.  See also CERT STR37-C's explanation.

Add our own *_c() functions operating on plain chars to hts_internal.h,
which also allows us to in future reimplement them directly to make them
immune to locales.

Use *_c() instead of casting to unsigned char in hts.c and vcf.c.
diff --git a/Makefile b/Makefile
@@ -258,7 +258,7 @@ hfile_irods.o hfile_irods.pico: hfile_irods.c config.h $(hfile_internal_h) $(hts
 hfile_libcurl.o hfile_libcurl.pico: hfile_libcurl.c config.h $(hfile_internal_h) $(htslib_hts_h) $(htslib_kstring_h)
 hfile_net.o hfile_net.pico: hfile_net.c config.h $(hfile_internal_h) $(htslib_knetfile_h)
 hts.o hts.pico: hts.c config.h version.h $(htslib_hts_h) $(htslib_bgzf_h) $(cram_h) $(htslib_hfile_h) $(htslib_khash_h) $(htslib_kseq_h) $(htslib_ksort_h) $(hts_internal_h)
-vcf.o vcf.pico: vcf.c config.h $(htslib_vcf_h) $(htslib_bgzf_h) $(htslib_tbx_h) $(htslib_hfile_h) $(htslib_khash_h) $(htslib_kseq_h) $(htslib_kstring_h) $(htslib_khash_str2int_h)
+vcf.o vcf.pico: vcf.c config.h $(htslib_vcf_h) $(htslib_bgzf_h) $(htslib_tbx_h) $(htslib_hfile_h) $(hts_internal_h) $(htslib_khash_h) $(htslib_kseq_h) $(htslib_kstring_h) $(htslib_khash_str2int_h)
 sam.o sam.pico: sam.c config.h $(htslib_sam_h) $(htslib_bgzf_h) $(cram_h) $(hts_internal_h) $(htslib_hfile_h) $(htslib_khash_h) $(htslib_kseq_h) $(htslib_kstring_h)
 tbx.o tbx.pico: tbx.c config.h $(htslib_tbx_h) $(htslib_bgzf_h) $(htslib_khash_h)
 faidx.o faidx.pico: faidx.c config.h $(htslib_bgzf_h) $(htslib_faidx_h) $(htslib_hfile_h) $(htslib_khash_h) $(htslib_kstring_h)
diff --git a/hts.c b/hts.c
@@ -431,7 +431,7 @@ scan_keyword(const char *str, char delim, char *buf, size_t buflen)
 {
     size_t i = 0;
     while (*str && *str != delim) {
-        if (i < buflen-1) buf[i++] = tolower((unsigned char) *str);
+        if (i < buflen-1) buf[i++] = tolower_c(*str);
         str++;
     }
 
diff --git a/hts_internal.h b/hts_internal.h
@@ -1,6 +1,6 @@
 /*  hts_internal.h -- internal functions; not part of the public API.
 
-    Copyright (C) 2015 Genome Research Ltd.
+    Copyright (C) 2015-2016 Genome Research Ltd.
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal
@@ -24,13 +24,34 @@ DEALINGS IN THE SOFTWARE.  */
 #define HTSLIB_HTS_INTERNAL_H
 
 #include <stddef.h>
+#include <ctype.h>
 
 #include "htslib/hts.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+// The <ctype.h> functions operate on ints such as are returned by fgetc(),
+// i.e., characters represented as unsigned-char-valued ints, or EOF.
+// To operate on plain chars (and to avoid warnings on some platforms),
+// technically one must cast to unsigned char everywhere (see CERT STR37-C)
+// or less painfully use these *_c() functions that operate on plain chars
+// (but not EOF, which must be considered separately where it is applicable).
+// TODO We may eventually wish to implement these functions directly without
+// using their <ctype.h> equivalents, and thus make them immune to locales.
+static inline int isalnum_c(char c) { return isalnum((unsigned char) c); }
+static inline int isalpha_c(char c) { return isalpha((unsigned char) c); }
+static inline int isdigit_c(char c) { return isdigit((unsigned char) c); }
+static inline int isgraph_c(char c) { return isgraph((unsigned char) c); }
+static inline int islower_c(char c) { return islower((unsigned char) c); }
+static inline int isprint_c(char c) { return isprint((unsigned char) c); }
+static inline int isspace_c(char c) { return isspace((unsigned char) c); }
+static inline int isupper_c(char c) { return isupper((unsigned char) c); }
+static inline char tolower_c(char c) { return tolower((unsigned char) c); }
+static inline char toupper_c(char c) { return toupper((unsigned char) c); }
+
+
 struct cram_fd;
 
 char *hts_idx_getfn(const char *fn, const char *ext);
diff --git a/vcf.c b/vcf.c
@@ -39,6 +39,7 @@ DEALINGS IN THE SOFTWARE.  */
 #include "htslib/tbx.h"
 #include "htslib/hfile.h"
 #include "htslib/khash_str2int.h"
+#include "hts_internal.h"
 
 #include "htslib/khash.h"
 KHASH_MAP_INIT_STR(vdict, bcf_idinfo_t)
@@ -1741,7 +1742,7 @@ static int vcf_parse_format(kstring_t *s, const bcf_hdr_t *h, bcf1_t *v, char *p
                 t++;
             }
             else {
-                fprintf(stderr,"[E::%s] Invalid character '%c' in '%s' FORMAT field at %s:%d\n", __FUNCTION__, isprint((unsigned char) *t)? *t : '?', h->id[BCF_DT_ID][z->key].key, bcf_seqname(h,v), v->pos+1);
+                fprintf(stderr,"[E::%s] Invalid character '%c' in '%s' FORMAT field at %s:%d\n", __FUNCTION__, isprint_c(*t)? *t : '?', h->id[BCF_DT_ID][z->key].key, bcf_seqname(h,v), v->pos+1);
                 // TODO Set v->errcode appropriately
                 return -1;
             }

Original file line number	Diff line number	Diff line change
`@@ -431,7 +431,7 @@ scan_keyword(const char str, char delim, char buf, size_t buflen)`
`431`	`431`	`{`
`432`	`432`	`size_t i = 0;`
`433`	`433`	`while (str && str != delim) {`
`434`		`- if (i < buflen-1) buf[i++] = tolower((unsigned char) *str);`
	`434`	`+ if (i < buflen-1) buf[i++] = tolower_c(*str);`
`435`	`435`	`str++;`
`436`	`436`	`}`
`437`	`437`