Remove use of sprintf() from HTSlib source #1594
Conversation
|
Code in test/*.c doesn't go into the library, so you could consider leaving those ones as is. |
| @@ -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); | |||
There was a problem hiding this comment.
This could probably just become %s.fai now with a size limit already enforced.
There was a problem hiding this comment.
It depends if you want to keep the .fai on the end...
As this involves PATH_MAX which may be removed later, I'm inclined to leave this as-is for now.
There was a problem hiding this comment.
Good point. I guess it's better if it truncates the pathname leaving .fai, as it prevents something catastrophic such as overwriting the main file.
(Although this looks like it's only loading, so it's just a case of preventing it from accidentally reading the wrong file on truncation)
| 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'; | ||
| } |
There was a problem hiding this comment.
I noticed this while looking at it a few days ago. It's an incorrect copy from escape_path, irrelevant here because it doesn't have the if (c == '?') break clause.
However it should probably get a mention in the commit message.
plugin.c
Outdated
| @@ -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 : ""); | |||
There was a problem hiding this comment.
Again we may as well change this to %s now we don't need the length protection.
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.
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.
Fixes #1586