threads / patch / 13864

patchprogress.c: avoid use of dynamic-sized array

Subject: [PATCH] progress.c: avoid use of dynamic-sized array

## tl;dr

7 messages between Jun 8, 2008 and Jun 8, 2008. Diffs are folded; open one to read it.

replies: 6people: 5as markdown or json

Boyd Lynn Gerber· Jun 8, 2008, 15:26 UTC · lore

Dynamically sized arrays are gcc and C99 construct. Using them hurts portability to older compilers, although using them is nice in this case it is not desirable. This patch removes the only use of the construct in stop_progress_msg(); the function is about writing out a single line of a message, and the existing callers of this function feed messages of only bounded size anyway, so use of dynamic array is simply overkill.

Signed-off-by: Boyd Lynn Gerber <gerberb@zenez.com>

-- Boyd Gerber <gerberb@zenez.com> ZENEZ 1042 East Fort Union #135, Midvale Utah 84047

---
        Developer's Certificate of Origin 1.1
        By making a contribution to this project, I certify that:
        (a) The contribution was created in whole or in part by me and I
            have the right to submit it under the open source license
            indicated in the file; or
        (b) The contribution is based upon previous work that, to the best
            of my knowledge, is covered under an appropriate open source
            license and I have the right under that license to submit that
            work with modifications, whether created in whole or in part
            by me, under the same open source license (unless I am
            permitted to submit under a different license), as indicated
            in the file; or
        (c) The contribution was provided directly to me by some other
            person who certified (a), (b) or (c) and I have not modified
            it.
        (d) I understand and agree that this project and the contribution
            are public and that a record of the contribution (including all
            personal information I submit with it, including my sign-off) is
            maintained indefinitely and may be redistributed consistent with
            this project or the open source license(s) involved.

--- progress.c

Show changes to progress.c +8 −3
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);
-- 
1.5.2.4


--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047
Johannes Schindelin· Jun 8, 2008, 19:30 UTC · re: Boyd Lynn Gerber · lore

Re: [PATCH] progress.c: avoid use of dynamic-sized array

Hi,
On Sun, 8 Jun 2008, Boyd Lynn Gerber wrote:
Show 15 quoted lines
> 
> Dynamically sized arrays are gcc and C99 construct.  Using them hurts
> portability to older compilers, although using them is nice in this case
> it is not desirable.  This patch removes the only use of the construct
> in stop_progress_msg(); the function is about writing out a single line
> of a message, and the existing callers of this function feed messages
> of only bounded size anyway, so use of dynamic array is simply overkill.
> 
> Signed-off-by: Boyd Lynn Gerber <gerberb@zenez.com>
> 
> --
> Boyd Gerber <gerberb@zenez.com>
> ZENEZ	1042 East Fort Union #135, Midvale Utah  84047
> 
> ---

Do you really want to have your mail signature in the commit message, particularly because...

Show 15 quoted lines
> [...]
>
>  	clear_progress_signal();
>  	free(progress->throughput);
> -- 
> 1.5.2.4
> 
> 
> --
> Boyd Gerber <gerberb@zenez.com>
> ZENEZ	1042 East Fort Union #135, Midvale Utah  84047
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
You repeat it at the end of the mail anyway?
The patch looks fine to me, though.

Ciao, Dscho

Boyd Lynn Gerber· Jun 8, 2008, 19:52 UTC · re: Johannes Schindelin · lore

Re: [PATCH] progress.c: avoid use of dynamic-sized array

On Sun, 8 Jun 2008, Johannes Schindelin wrote:
Show 34 quoted lines
> On Sun, 8 Jun 2008, Boyd Lynn Gerber wrote:
> > Dynamically sized arrays are gcc and C99 construct.  Using them hurts
> > portability to older compilers, although using them is nice in this case
> > it is not desirable.  This patch removes the only use of the construct
> > in stop_progress_msg(); the function is about writing out a single line
> > of a message, and the existing callers of this function feed messages
> > of only bounded size anyway, so use of dynamic array is simply overkill.
> > 
> > Signed-off-by: Boyd Lynn Gerber <gerberb@zenez.com>
> > 
> > --
> > Boyd Gerber <gerberb@zenez.com>
> > ZENEZ	1042 East Fort Union #135, Midvale Utah  84047
> > 
> > ---
> 
> Do you really want to have your mail signature in the commit message, 
> particularly because...
> 
> > [...]
> >
> >  	clear_progress_signal();
> >  	free(progress->throughput);
> > -- 
> > 1.5.2.4
> > 
> > 
> > --
> > Boyd Gerber <gerberb@zenez.com>
> > ZENEZ	1042 East Fort Union #135, Midvale Utah  84047
> 
> You repeat it at the end of the mail anyway?
> 
> The patch looks fine to me, though.

I put it in because in the git repo it is not their. Only in the email. It is fine to remove it.

-- Boyd Gerber <gerberb@zenez.com> ZENEZ 1042 East Fort Union #135, Midvale Utah 84047

Alex Riesen· Jun 8, 2008, 21:34 UTC · re: Boyd Lynn Gerber · lore

Re: [PATCH] progress.c: avoid use of dynamic-sized array

Boyd Lynn Gerber, Sun, Jun 08, 2008 17:26:15 +0200:
Show 7 quoted lines
>  		/* 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);

It is just a temporary buffer, there is no urgent need to micro-optimize the allocation like that. Maybe just leave one of the buffers, the one on stack? It is just a progress message, snprintf will cut it, yes, but it is unlikely to cause any harm (unless you forget to \n-terminate it).

しらいしななこ· Jun 8, 2008, 21:50 UTC · re: Boyd Lynn Gerber · lore

Re: [PATCH] progress.c: avoid use of dynamic-sized array

Quoting Boyd Lynn Gerber <gerberb@zenez.com>:
Show 8 quoted lines
> Dynamically sized arrays are gcc and C99 construct.  Using them hurts
> portability to older compilers, although using them is nice in this case
> it is not desirable.  This patch removes the only use of the construct
> in stop_progress_msg(); the function is about writing out a single line
> of a message, and the existing callers of this function feed messages
> of only bounded size anyway, so use of dynamic array is simply overkill.
>
> Signed-off-by: Boyd Lynn Gerber <gerberb@zenez.com>

I may be mistaken but isn't this Junio's patch? If so (quoting from SubmittingPatches document):

  If you are forwarding a patch from somebody else, optionally, at
  the beginning of the e-mail message just before the commit
  message starts, you can put a "From: " line to name that person.
-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

----------------------------------------------------------------------
Find out how you can get spam free email.
http://www.bluebottle.com/tag/3
Junio C Hamano· Jun 8, 2008, 22:02 UTC · re: しらいしななこ · lore

Re: [PATCH] progress.c: avoid use of dynamic-sized array

しらいしななこ  <nanako3@bluebottle.com> writes:
Show 17 quoted lines
> Quoting Boyd Lynn Gerber <gerberb@zenez.com>:
>
>> Dynamically sized arrays are gcc and C99 construct.  Using them hurts
>> portability to older compilers, although using them is nice in this case
>> it is not desirable.  This patch removes the only use of the construct
>> in stop_progress_msg(); the function is about writing out a single line
>> of a message, and the existing callers of this function feed messages
>> of only bounded size anyway, so use of dynamic array is simply overkill.
>>
>> Signed-off-by: Boyd Lynn Gerber <gerberb@zenez.com>
>
> I may be mistaken but isn't this Junio's patch?  If so (quoting
> from SubmittingPatches document):
>
>   If you are forwarding a patch from somebody else, optionally, at
>   the beginning of the e-mail message just before the commit
>   message starts, you can put a "From: " line to name that person.
Heh, thanks for being picky ;-)

Something this small, either way is fine by me. Besides, I've applied the patch (and the other one) from Boyd already.

Boyd Lynn Gerber· Jun 8, 2008, 21:53 UTC · lore

Re: [PATCH] progress.c: avoid use of dynamic-sized array

On Mon, 9 Jun 2008, しらいしななこ wrote:
Show 16 quoted lines
> Quoting Boyd Lynn Gerber <gerberb@zenez.com>:
> > Dynamically sized arrays are gcc and C99 construct.  Using them hurts
> > portability to older compilers, although using them is nice in this case
> > it is not desirable.  This patch removes the only use of the construct
> > in stop_progress_msg(); the function is about writing out a single line
> > of a message, and the existing callers of this function feed messages
> > of only bounded size anyway, so use of dynamic array is simply overkill.
> >
> > Signed-off-by: Boyd Lynn Gerber <gerberb@zenez.com>
> 
> I may be mistaken but isn't this Junio's patch?  If so (quoting
> from SubmittingPatches document):
> 
>   If you are forwarding a patch from somebody else, optionally, at
>   the beginning of the e-mail message just before the commit
>   message starts, you can put a "From: " line to name that person.

It is his from my orignal modifications. I thought he suggested I make the changes and resubmit it as a seperate patch. Thanks for the clarifications.

-- Boyd Gerber <gerberb@zenez.com> ZENEZ 1042 East Fort Union #135, Midvale Utah 84047

← back to recent threads