Skip to content

Commit

Permalink
Remove use of sprintf() from HTSlib source
Browse files Browse the repository at this point in the history
None of these instances were really a problem, but using it
upsets some downstream packagers (notably R).  The easiest
way to keep them happy is to stop using it and (mostly) switch
to snprintf() instead.

Also remove some code from hfile_s3's escape_query() which
could never be executed.
  • Loading branch information
daviesrob authored and jkbonfield committed Mar 30, 2023
1 parent c1634e7 commit ffd74ec
Show file tree
Hide file tree
Showing 13 changed files with 87 additions and 85 deletions.
21 changes: 12 additions & 9 deletions cram/cram_index.c
Original file line number Diff line number Diff line change
Expand Up @@ -656,9 +656,10 @@ static int cram_index_build_multiref(cram_fd *fd,
}

if (ref != -2) {
sprintf(buf, "%d\t%"PRId64"\t%"PRId64"\t%"PRId64"\t%d\t%d\n",
ref, ref_start, ref_end - ref_start + 1,
(int64_t)cpos, landmark, sz);
snprintf(buf, sizeof(buf),
"%d\t%"PRId64"\t%"PRId64"\t%"PRId64"\t%d\t%d\n",
ref, ref_start, ref_end - ref_start + 1,
(int64_t)cpos, landmark, sz);
if (bgzf_write(fp, buf, strlen(buf)) < 0)
return -4;
}
Expand All @@ -669,9 +670,10 @@ static int cram_index_build_multiref(cram_fd *fd,
}

if (ref != -2) {
sprintf(buf, "%d\t%"PRId64"\t%"PRId64"\t%"PRId64"\t%d\t%d\n",
ref, ref_start, ref_end - ref_start + 1,
(int64_t)cpos, landmark, sz);
snprintf(buf, sizeof(buf),
"%d\t%"PRId64"\t%"PRId64"\t%"PRId64"\t%d\t%d\n",
ref, ref_start, ref_end - ref_start + 1,
(int64_t)cpos, landmark, sz);
if (bgzf_write(fp, buf, strlen(buf)) < 0)
return -4;
}
Expand Down Expand Up @@ -701,9 +703,10 @@ int cram_index_slice(cram_fd *fd,
if (s->hdr->ref_seq_id == -2) {
ret = cram_index_build_multiref(fd, c, s, fp, cpos, spos, sz);
} else {
sprintf(buf, "%d\t%"PRId64"\t%"PRId64"\t%"PRId64"\t%d\t%d\n",
s->hdr->ref_seq_id, s->hdr->ref_seq_start,
s->hdr->ref_seq_span, (int64_t)cpos, (int)spos, (int)sz);
snprintf(buf, sizeof(buf),
"%d\t%"PRId64"\t%"PRId64"\t%"PRId64"\t%d\t%d\n",
s->hdr->ref_seq_id, s->hdr->ref_seq_start,
s->hdr->ref_seq_span, (int64_t)cpos, (int)spos, (int)sz);
ret = (bgzf_write(fp, buf, strlen(buf)) >= 0)? 0 : -4;
}

Expand Down
6 changes: 3 additions & 3 deletions cram/cram_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -2531,7 +2531,7 @@ static refs_t *refs_load_fai(refs_t *r_orig, const char *fn, int is_err) {
/* Only the reference file provided. Get the index file name from it */
if (!(r->fn = string_dup(r->pool, fn)))
goto err;
sprintf(fai_fn, "%.*s.fai", PATH_MAX-5, fn);
snprintf(fai_fn, PATH_MAX, "%.*s.fai", PATH_MAX-5, fn);
}
}

Expand Down Expand Up @@ -4816,7 +4816,7 @@ static void full_path(char *out, char *in) {
strncpy(out, in, PATH_MAX-1);
out[PATH_MAX-1] = 0;
} else {
int len;
size_t len;

// unable to get dir or out+in is too long
if (!getcwd(out, PATH_MAX) ||
Expand All @@ -4826,7 +4826,7 @@ static void full_path(char *out, char *in) {
return;
}

sprintf(out+len, "/%.*s", PATH_MAX - 2 - len, in);
snprintf(out+len, PATH_MAX - len, "/%s", in);

// FIXME: cope with `pwd`/../../../foo.fa ?
}
Expand Down
2 changes: 1 addition & 1 deletion cram/open_trace_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ static char *expand_path(const char *file, char *dirname, int max_s_digits) {

/* Special case for "./" or absolute filenames */
if (*file == '/' || (len==1 && *dirname == '.')) {
sprintf(path, "%s", file);
memcpy(path, file, lenf + 1);
} else {
/* Handle %[0-9]*s expansions, if required */
char *path_end = path;
Expand Down
40 changes: 18 additions & 22 deletions hfile_s3.c
Original file line number Diff line number Diff line change
Expand Up @@ -451,12 +451,12 @@ static int auth_header_callback(void *ctx, char ***hdrs) {

/* like a escape path but for query strings '=' and '&' are untouched */
static char *escape_query(const char *qs) {
size_t i, j = 0, length;
size_t i, j = 0, length, alloced;
char *escaped;

length = strlen(qs);

if ((escaped = malloc(length * 3 + 1)) == NULL) {
alloced = length * 3 + 1;
if ((escaped = malloc(alloced)) == NULL) {
return NULL;
}

Expand All @@ -467,29 +467,25 @@ static char *escape_query(const char *qs) {
c == '_' || c == '-' || c == '~' || c == '.' || c == '/' || c == '=' || c == '&') {
escaped[j++] = c;
} else {
sprintf(escaped + j, "%%%02X", c);
snprintf(escaped + j, alloced - j, "%%%02X", c);
j += 3;
}
}

if (i != length) {
// in the case of a '?' copy the rest of the qs across unchanged
strcpy(escaped + j, qs + i);
} else {
escaped[j] = '\0';
}
escaped[j] = '\0';

return escaped;
}


static char *escape_path(const char *path) {
size_t i, j = 0, length;
size_t i, j = 0, length, alloced;
char *escaped;

length = strlen(path);
alloced = length * 3 + 1;

if ((escaped = malloc(length * 3 + 1)) == NULL) {
if ((escaped = malloc(alloced)) == NULL) {
return NULL;
}

Expand All @@ -502,7 +498,7 @@ static char *escape_path(const char *path) {
c == '_' || c == '-' || c == '~' || c == '.' || c == '/') {
escaped[j++] = c;
} else {
sprintf(escaped + j, "%%%02X", c);
snprintf(escaped + j, alloced - j, "%%%02X", c);
j += 3;
}
}
Expand Down Expand Up @@ -842,14 +838,14 @@ AWS S3 sig version 4 writing code
****************************************************************/

static void hash_string(char *in, size_t length, char *out) {
static void hash_string(char *in, size_t length, char *out, size_t out_len) {
unsigned char hashed[SHA256_DIGEST_BUFSIZE];
int i, j;

s3_sha256((const unsigned char *)in, length, hashed);

for (i = 0, j = 0; i < SHA256_DIGEST_BUFSIZE; i++, j+= 2) {
sprintf(out + j, "%02x", hashed[i]);
snprintf(out + j, out_len - j, "%02x", hashed[i]);
}
}

Expand All @@ -866,7 +862,7 @@ static void ksfree(kstring_t *s) {
}


static int make_signature(s3_auth_data *ad, kstring_t *string_to_sign, char *signature_string) {
static int make_signature(s3_auth_data *ad, kstring_t *string_to_sign, char *signature_string, size_t sig_string_len) {
unsigned char date_key[SHA256_DIGEST_BUFSIZE];
unsigned char date_region_key[SHA256_DIGEST_BUFSIZE];
unsigned char date_region_service_key[SHA256_DIGEST_BUFSIZE];
Expand All @@ -893,7 +889,7 @@ static int make_signature(s3_auth_data *ad, kstring_t *string_to_sign, char *sig
s3_sign_sha256(signing_key, len, (const unsigned char *)string_to_sign->s, string_to_sign->l, signature, &len);

for (i = 0, j = 0; i < len; i++, j+= 2) {
sprintf(signature_string + j, "%02x", signature[i]);
snprintf(signature_string + j, sig_string_len - j, "%02x", signature[i]);
}

ksfree(&secret_access_key);
Expand Down Expand Up @@ -945,7 +941,7 @@ static int make_authorisation(s3_auth_data *ad, char *http_request, char *conten
goto cleanup;
}

hash_string(canonical_request.s, canonical_request.l, cr_hash);
hash_string(canonical_request.s, canonical_request.l, cr_hash, sizeof(cr_hash));

ksprintf(&scope, "%s/%s/s3/aws4_request", ad->date_short, ad->region.s);

Expand All @@ -959,7 +955,7 @@ static int make_authorisation(s3_auth_data *ad, char *http_request, char *conten
goto cleanup;
}

if (make_signature(ad, &string_to_sign, signature_string)) {
if (make_signature(ad, &string_to_sign, signature_string, sizeof(signature_string))) {
goto cleanup;
}

Expand Down Expand Up @@ -1094,10 +1090,10 @@ static int write_authorisation_callback(void *auth, char *request, kstring_t *co
}

if (content) {
hash_string(content->s, content->l, content_hash);
hash_string(content->s, content->l, content_hash, sizeof(content_hash));
} else {
// empty hash
hash_string("", 0, content_hash);
hash_string("", 0, content_hash, sizeof(content_hash));
}

ad->canonical_query_string.l = 0;
Expand Down Expand Up @@ -1166,7 +1162,7 @@ static int v4_auth_header_callback(void *ctx, char ***hdrs) {
return copy_auth_headers(ad, hdrs);
}

hash_string("", 0, content_hash); // empty hash
hash_string("", 0, content_hash, sizeof(content_hash)); // empty hash

ad->canonical_query_string.l = 0;

Expand Down
2 changes: 1 addition & 1 deletion kstring.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ int kputd(double d, kstring_t *s) {
if (ks_resize(s, s->l + 50) < 0)
return EOF;
// We let stdio handle the exponent cases
int s2 = sprintf(s->s + s->l, "%g", d);
int s2 = snprintf(s->s + s->l, s->m - s->l, "%g", d);
len += s2;
s->l += s2;
return len;
Expand Down
2 changes: 1 addition & 1 deletion plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ const char *hts_plugin_path(void) {
}

static char s_path[1024];
sprintf(s_path, "%.1023s", ks.s ? ks.s : "");
snprintf(s_path, sizeof(s_path), "%s", ks.s ? ks.s : "");
free(ks.s);

return s_path;
Expand Down
20 changes: 11 additions & 9 deletions sam.c
Original file line number Diff line number Diff line change
Expand Up @@ -5392,20 +5392,22 @@ int bam_plp_insertion_mod(const bam_pileup1_t *p,
for (j = 0; j < nm; j++) {
char qual[20];
if (mod[j].qual >= 0)
sprintf(qual, "%d", mod[j].qual);
snprintf(qual, sizeof(qual), "%d", mod[j].qual);
else
*qual=0;
if (mod[j].modified_base < 0)
// ChEBI
indel += sprintf(&ins->s[indel], "%c(%d)%s",
"+-"[mod[j].strand],
-mod[j].modified_base,
qual);
indel += snprintf(&ins->s[indel], ins->m - indel,
"%c(%d)%s",
"+-"[mod[j].strand],
-mod[j].modified_base,
qual);
else
indel += sprintf(&ins->s[indel], "%c%c%s",
"+-"[mod[j].strand],
mod[j].modified_base,
qual);
indel += snprintf(&ins->s[indel], ins->m - indel,
"%c%c%s",
"+-"[mod[j].strand],
mod[j].modified_base,
qual);
}
ins->s[indel++] = ']';
ins->l += indel - o_indel; // grow by amount we used
Expand Down
2 changes: 1 addition & 1 deletion test/hfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ int main(void)
original = slurp("vcf.c");
for (i = 1; i <= 6; i++) {
char *text;
sprintf(buffer, "test/hfile%d.tmp", i);
snprintf(buffer, sizeof(buffer), "test/hfile%d.tmp", i);
text = slurp(buffer);
if (strcmp(original, text) != 0) {
fprintf(stderr, "%s differs from vcf.c\n", buffer);
Expand Down
2 changes: 1 addition & 1 deletion test/sam.c
Original file line number Diff line number Diff line change
Expand Up @@ -1504,7 +1504,7 @@ static void faidx1(const char *filename)

fin = fopen(filename, "rb");
if (fin == NULL) fail("can't open %s", filename);
sprintf(tmpfilename, "%s.tmp", filename);
snprintf(tmpfilename, sizeof(tmpfilename), "%s.tmp", filename);
fout = fopen(tmpfilename, "wb");
if (fout == NULL) fail("can't create temporary %s", tmpfilename);
while (fgets(line, sizeof line, fin)) {
Expand Down
22 changes: 11 additions & 11 deletions test/test-regidx.c
Original file line number Diff line number Diff line change
Expand Up @@ -304,20 +304,20 @@ void test_explicit(char *tgt, char *qry, char *exp)
regidx_destroy(idx);
}

void create_line_bed(char *line, char *chr, int start, int end)
void create_line_bed(char *line, size_t size, char *chr, int start, int end)
{
sprintf(line,"%s\t%d\t%d\n",chr,start-1,end);
snprintf(line,size,"%s\t%d\t%d\n",chr,start-1,end);
}
void create_line_tab(char *line, char *chr, int start, int end)
void create_line_tab(char *line, size_t size, char *chr, int start, int end)
{
sprintf(line,"%s\t%d\t%d\n",chr,start,end);
snprintf(line,size,"%s\t%d\t%d\n",chr,start,end);
}
void create_line_reg(char *line, char *chr, int start, int end)
void create_line_reg(char *line, size_t size, char *chr, int start, int end)
{
sprintf(line,"%s:%d-%d\n",chr,start,end);
snprintf(line,size,"%s:%d-%d\n",chr,start,end);
}

typedef void (*set_line_f)(char *line, char *chr, int start, int end);
typedef void (*set_line_f)(char *line, size_t size, char *chr, int start, int end);

void test(set_line_f set_line, regidx_parse_f parse)
{
Expand All @@ -329,17 +329,17 @@ void test(set_line_f set_line, regidx_parse_f parse)
for (i=1; i<n; i++)
{
start = end = 10*i;
set_line(line,chr,start,end);
set_line(line,sizeof(line),chr,start,end);
debug("insert: %s", line);
if ( regidx_insert(idx,line)!=0 ) error("insert failed: %s\n", line);

start = end = 10*i + 1;
set_line(line,chr,start,end);
set_line(line,sizeof(line),chr,start,end);
debug("insert: %s", line);
if ( regidx_insert(idx,line)!=0 ) error("insert failed: %s\n", line);

start = 20000*i; end = start + 2000;
set_line(line,chr,start,end);
set_line(line,sizeof(line),chr,start,end);
debug("insert: %s", line);
if ( regidx_insert(idx,line)!=0 ) error("insert failed: %s\n", line);
}
Expand Down Expand Up @@ -396,7 +396,7 @@ void test(set_line_f set_line, regidx_parse_f parse)

// fully contained interval, one hit
start = 20000*i - 5000; end = 20000*i + 3000;
set_line(line,chr,start,end);
set_line(line,sizeof(line),chr,start,end);
if ( !regidx_overlap(idx,chr,start-1,end-1,itr) ) error("query failed, there should be a hit: %s:%d-%d\n",chr,start,end);
debug("ok: overlap(s) found for %s:%d-%d\n",chr,start,end);
nhit = 0;
Expand Down
Loading

0 comments on commit ffd74ec

Please sign in to comment.