Format time_t values portably and fix other values too #1085
Conversation
src/util.c
Outdated
| if (!string_nformat(buf, buflen, NULL, "%s%"PRIdMAX"%c", | ||
| now.tv_sec >= timestamp ? "" : "-", | ||
| seconds, reldate[i].compact_symbol)) | ||
| return ""; | ||
|
|
||
| } else if (!string_nformat(buf, buflen, NULL, "%ld %s%s %s", | ||
| } else if (!string_nformat(buf, buflen, NULL, "%"PRIdMAX" %s%s %s", |
There was a problem hiding this comment.
If you use this format specifier, what you should do is cast the value as well (so (intmax_t)value iirc). Otherwise this will read 64 bits from a 32 bit field on 32bit glibc, for example.
Generally speaking, I'd recommend doing %lld for format and (long long)value instead.
This goes for all instances.
There was a problem hiding this comment.
Agreed, let's go the BSD way.
I actually took example on Git but I overlooked that they define their own timestamp_t type (though there was a clue as they use the unsigned version PRIuMAX).
| @@ -170,7 +170,7 @@ file_finder_draw(struct file_finder *finder) | |||
|
|
|||
| wmove(finder->win, finder->height - 1, 0); | |||
| wbkgdset(finder->win, get_line_attr(NULL, LINE_TITLE_FOCUS)); | |||
| wprintw(finder->win, "[finder] file %d of %d", pos->lineno + 1, finder->lines); | |||
| wprintw(finder->win, "[finder] file %lu of %zu", pos->lineno + 1, finder->lines); | |||
There was a problem hiding this comment.
In https://github.com/jonas/tig/pull/1085/files#diff-662ec1e8c57906cca33cf41b09c520a818a03e8c7454538e3529869b8c2b24b4R693 you're using %u for line->lineno. Do the lineno members have different types?
There was a problem hiding this comment.
Yes, I noticed that too, lineno is unsigned int in line struct while it is unsigned long in position struct. For now, I only wanted to restore the concordance between formats and types, but I agree it could deserve attention as well.
src/draw.c
Outdated
| @@ -322,7 +322,7 @@ draw_lineno_custom(struct view *view, struct view_column *column, unsigned int l | |||
| return false; | |||
|
|
|||
| if (lineno == 1 || (lineno % interval) == 0) { | |||
| static char fmt[] = "%ld"; | |||
| static char fmt[] = "%#u"; | |||
There was a problem hiding this comment.
I'm reasonably sure u can't use #. GCC even warns about it.
There was a problem hiding this comment.
Actually # is just a placeholder, it is replaced with a digit one line below. I'll use %3u for clarity.
|
Thanks for the review @ericonr, I was sure we could use more than one pair of eyes on this kind of subject :-). |
koutcher
force-pushed
the
gh-1084-format-time_t-portably
branch
from
28ba357 to
31f1d32
Compare
Fixes #1084