Skip to content

Commit b3faeb3

Browse files
committed
Ensure indirect function calls have the correct type.
Some HTSlib interfaces (notably kgetline(), kgetline2(), hts_itr_querys() and hts_parse_region()) have function callbacks that are intended to be generic, so the function signatures include a `void *` for data to be passed in. E.g. for kgetline() this would be the FILE * or hFILE * to read. Historically they have often been called with a function that has an explicit type (e.g. FILE * for fgets() used with kgetline) on the assumption that casting it to void * is harmless. Unfortunately doing this is strictly undefined behaviour and will fail in some conditions - for example if compiling with control flow integrity turned on. Recent versions of clang will also call this out when using undefined behaviour sanitizer. The solution is to create wrapper functions that do take a void *, and call the intended function with suitable casts. Some new interfaces are also added so that the messy implementation details can be hidden away, and the inputs can be better type checked. Hopefully this should also allow the wrapper functions to be inlined away. * khgetline() to read lines from an hFILE * to a kstring. * kfgetline() to read lines from a FILE * to a kstring. * tbx_itr_querys1() to create a tabix iterator on a region. Used to reimplement the existing tbx_itr_querys() macro. * bcf_itr_querys1() to create a region iterator an a bcf file Used to reimplement the existing bcf_itr_querys() macro.
1 parent b13f80a commit b3faeb3

14 files changed

Lines changed: 97 additions & 15 deletions

File tree

faidx.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,7 @@ const char *fai_parse_region(const faidx_t *fai, const char *s,
10221022
int *tid, hts_pos_t *beg, hts_pos_t *end,
10231023
int flags)
10241024
{
1025-
return hts_parse_region(s, tid, beg, end, (hts_name2id_f)fai_name2id, (void *)fai, flags);
1025+
return hts_parse_region(s, tid, beg, end, fai_name2id, (void *)fai, flags);
10261026
}
10271027

10281028
void fai_set_cache_size(faidx_t *fai, int cache_size) {

header.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1439,7 +1439,7 @@ int sam_hdr_build_from_sam_file(sam_hdr_t *hdr, htsFile* fp) {
14391439
if (f == NULL)
14401440
goto error;
14411441

1442-
while (line.l = 0, kgetline(&line, (kgets_func*) hgets, f) >= 0) {
1442+
while (line.l = 0, khgetline(&line, f) >= 0) {
14431443
char* tab = strchr(line.s, '\t');
14441444
size_t l_start = str.l;
14451445
sam_hrec_type_t *h_type;

hfile.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,19 @@ char *hgets(char *buffer, int size, hFILE *fp)
297297
return hgetln(buffer, size, fp) > 0 ? buffer : NULL;
298298
}
299299

300+
// Wrap around hgets() to get the right signature for kgets_func
301+
static char *hgets_wrapper(char *buffer, int size, void *fp)
302+
{
303+
return hgets(buffer, size, (hFILE *) fp);
304+
}
305+
306+
int khgetline(struct kstring_t *kstr, hFILE *fp)
307+
{
308+
if (!kstr || !fp)
309+
return EOF;
310+
return kgetline(kstr, hgets_wrapper, fp);
311+
}
312+
300313
ssize_t hpeek(hFILE *fp, void *buffer, size_t nbytes)
301314
{
302315
size_t n = fp->end - fp->begin;

hfile_libcurl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ static int read_auth_plain(auth_token *tok, hFILE *auth_fp) {
517517
kstring_t token = {0, 0, NULL};
518518
const char *start, *end;
519519

520-
if (kgetline(&line, (char * (*)(char *, int, void *)) hgets, auth_fp) < 0) goto error;
520+
if (khgetline(&line, auth_fp) < 0) goto error;
521521
if (kputc('\0', &line) < 0) goto error;
522522

523523
for (start = line.s; *start && isspace_c(*start); start++) {}

hfile_s3.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,6 @@ static FILE *expand_tilde_open(const char *fname, const char *mode)
249249
return fp;
250250
}
251251

252-
253252
static void parse_ini(const char *fname, const char *section, ...)
254253
{
255254
kstring_t line = { 0, 0, NULL };
@@ -259,7 +258,7 @@ static void parse_ini(const char *fname, const char *section, ...)
259258
FILE *fp = expand_tilde_open(fname, "r");
260259
if (fp == NULL) return;
261260

262-
while (line.l = 0, kgetline(&line, (kgets_func *) fgets, fp) >= 0)
261+
while (line.l = 0, kfgetline(&line, fp) >= 0)
263262
if (line.s[0] == '[' && (s = strchr(line.s, ']')) != NULL) {
264263
*s = '\0';
265264
active = (strcmp(&line.s[1], section) == 0);
@@ -301,7 +300,7 @@ static void parse_simple(const char *fname, kstring_t *id, kstring_t *secret)
301300
FILE *fp = expand_tilde_open(fname, "r");
302301
if (fp == NULL) return;
303302

304-
while (kgetline(&text, (kgets_func *) fgets, fp) >= 0)
303+
while (kfgetline(&text, fp) >= 0)
305304
kputc(' ', &text);
306305
fclose(fp);
307306

hts.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2027,6 +2027,11 @@ off_t hts_utell(htsFile *fp)
20272027
return htell(fp->fp.hfile);
20282028
}
20292029

2030+
// Wrap hgetln() with a kgets_func2 signature for kgetline2()
2031+
static ssize_t hgetln_wrapper(char *buf, size_t len, void *vfp) {
2032+
return hgetln(buf, len, (hFILE *) vfp);
2033+
}
2034+
20302035
int hts_getline(htsFile *fp, int delimiter, kstring_t *str)
20312036
{
20322037
int ret;
@@ -2038,7 +2043,7 @@ int hts_getline(htsFile *fp, int delimiter, kstring_t *str)
20382043
switch (fp->format.compression) {
20392044
case no_compression:
20402045
str->l = 0;
2041-
ret = kgetline2(str, (kgets_func2 *) hgetln, fp->fp.hfile);
2046+
ret = kgetline2(str, hgetln_wrapper, fp->fp.hfile);
20422047
if (ret >= 0) ret = (str->l <= INT_MAX)? (int) str->l : INT_MAX;
20432048
else if (herrno(fp->fp.hfile)) ret = -2, errno = herrno(fp->fp.hfile);
20442049
else ret = -1;

htslib/hfile.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,19 @@ kstring's `kgetline()` to read arbitrarily-long lines into a _kstring_t_.
211211
HTSLIB_EXPORT
212212
char *hgets(char *buffer, int size, hFILE *fp) HTS_RESULT_USED;
213213

214+
/// Read a line from a stream and append it to a kstring
215+
/** @param kstr The destination kstring
216+
@param fp The file stream
217+
@return 0 on success; EOF on end of file or if an error occurred.
218+
@since 1.24
219+
220+
Reads a "\n"- or "\r\n"- terminated line from fp into kstr.
221+
The line read is appended without its terminator and 0 is returned;
222+
EOF is returned at end of file or on error.
223+
*/
224+
HTSLIB_EXPORT
225+
int khgetline(struct kstring_t *kstr, hFILE *fp) HTS_RESULT_USED;
226+
214227
/// Peek at characters to be read without removing them from buffers
215228
/** @param fp The file stream
216229
@param buffer The buffer to which the peeked bytes will be written

htslib/kstring.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ extern "C" {
129129
HTSLIB_EXPORT
130130
int kgetline(kstring_t *s, kgets_func *fgets_fn, void *fp);
131131

132+
/* Convenience function to call kgetline with a FILE * */
133+
HTSLIB_EXPORT
134+
int kfgetline(kstring_t *s, FILE *fp);
135+
132136
/* kgetline2() uses the supplied hgetln()-like function to read a "\n"-
133137
* or "\r\n"-terminated line from fp. The line read is appended to the
134138
* ksring without its terminator and 0 is returned; EOF is returned at

htslib/tbx.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,13 @@ extern const tbx_conf_t tbx_conf_gff, tbx_conf_bed, tbx_conf_psltbl, tbx_conf_sa
5858

5959
#define tbx_itr_destroy(iter) hts_itr_destroy(iter)
6060
#define tbx_itr_queryi(tbx, tid, beg, end) hts_itr_query((tbx)->idx, (tid), (beg), (end), tbx_readrec)
61-
#define tbx_itr_querys(tbx, s) hts_itr_querys((tbx)->idx, (s), (hts_name2id_f)(tbx_name2id), (tbx), hts_itr_query, tbx_readrec)
61+
#define tbx_itr_querys(tbx, s) tbx_itr_querys1((tbx), (s))
6262
#define tbx_itr_next(htsfp, tbx, itr, r) hts_itr_next(hts_get_bgzfp(htsfp), (itr), (r), (tbx))
6363
#define tbx_bgzf_itr_next(bgzfp, tbx, itr, r) hts_itr_next((bgzfp), (itr), (r), (tbx))
6464

65+
HTSLIB_EXPORT
66+
hts_itr_t *tbx_itr_querys1(tbx_t *tbx, const char *region);
67+
6568
HTSLIB_EXPORT
6669
int tbx_name2id(tbx_t *tbx, const char *ss);
6770

htslib/vcf.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1318,7 +1318,11 @@ set to one of BCF_ERR* codes and must be checked before calling bcf_write().
13181318

13191319
#define bcf_itr_destroy(iter) hts_itr_destroy(iter)
13201320
#define bcf_itr_queryi(idx, tid, beg, end) hts_itr_query((idx), (tid), (beg), (end), bcf_readrec)
1321-
#define bcf_itr_querys(idx, hdr, s) hts_itr_querys((idx), (s), (hts_name2id_f)(bcf_hdr_name2id), (hdr), hts_itr_query, bcf_readrec)
1321+
#define bcf_itr_querys(idx, hdr, s) bcf_itr_querys1((idx), (hdr), (s))
1322+
1323+
HTSLIB_EXPORT
1324+
hts_itr_t *bcf_itr_querys1(const hts_idx_t *idx, bcf_hdr_t *hdr,
1325+
const char *region);
13221326

13231327
static inline int bcf_itr_next(htsFile *htsfp, hts_itr_t *itr, void *r) {
13241328
if (htsfp->is_bgzf)

0 commit comments

Comments
 (0)