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.
  • Loading branch information
daviesrob committed Mar 29, 2023
1 parent c1634e7 commit 92a3fe0
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), "%.1023s", 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 92a3fe0

Please sign in to comment.