Boyd Lynn Gerber [off-list ref] writes:
quoted hunk ↗ jump to hunk
diff --git a/progress.c b/progress.c
index d19f80c..295c4e3 100644
--- a/progress.c
+++ b/progress.c
@@ -241,7 +241,8 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
*p_progress = NULL;
if (progress->last_value != -1) {
/* Force the last update */
- char buf[strlen(msg) + 5];
+ /* char buf[strlen(msg) + 5]; */
+ char *buf = alloca (strlen(msg) + 5 );
struct throughput *tp = progress->throughput;
if (tp) {
unsigned int rate = !tp->avg_misecs ? 0 :
I do not know the situation over there these days, but I have a distant
but bitter memory of having to deal with AIX X-<. It insisted that
inclusion of <alloca.h> to be the very first thing in the source before
anything else. I would want to keep alloca() out of the codebase without
very good reason. Not that I care much about portability to AIX, but not
having to worry about alloca() unless necessary is a good thing.
I do not think progress_msg() is a good reason to even worrying about a
dynamically sized array. The function is designed to spit out a single
line of message (so the incoming msg is expected to be shorter than 80
chars or so). If you "git grep stop_progress_msg", you will see that
there are only two callers of this function, one in progress.c itself that
says "done", and the other one in index-pack.c that gives a string
formatted into 48-byte buffer.
So we can be lazy and say:
char buf[128];
...
snprintf(buf, sizeof(buf), ", %s.\n", msg)
and be done with it.
If you really wanted to be safe and anal, you could do something like
this, which would be just as efficient and much more straightforward:
progress.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/progress.c b/progress.c
index d19f80c..55a8687 100644
--- a/progress.c
+++ b/progress.c
@@ -241,16 +241,21 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
*p_progress = NULL;
if (progress->last_value != -1) {
/* Force the last update */
- char buf[strlen(msg) + 5];
+ char buf[128], *bufp;
+ size_t len = strlen(msg) + 5;
struct throughput *tp = progress->throughput;
+
+ bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);
if (tp) {
unsigned int rate = !tp->avg_misecs ? 0 :
tp->avg_bytes / tp->avg_misecs;
throughput_string(tp, tp->curr_total, rate);
}
progress_update = 1;
- sprintf(buf, ", %s.\n", msg);
- display(progress, progress->last_value, buf);
+ sprintf(bufp, ", %s.\n", msg);
+ display(progress, progress->last_value, bufp);
+ if (buf != bufp)
+ free(bufp);
}
clear_progress_signal();
free(progress->throughput);