Patchwork chg: check snprintf result strictly

login
register
mail settings
Submitter Jun Wu
Date Jan. 11, 2017, 3:42 p.m.
Message ID <5cbdc769201b22cbc4f6.1484149377@x1c>
Download mbox | patch
Permalink /patch/18182/
State Accepted
Headers show

Comments

Jun Wu - Jan. 11, 2017, 3:42 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1484149164 -28800
#      Wed Jan 11 23:39:24 2017 +0800
# Node ID 5cbdc769201b22cbc4f698c7bc9235984c7cf427
# Parent  e882c7bb5a0ba2589a44108c9a87b300a13e08df
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 5cbdc769201b
chg: check snprintf result strictly

This makes the program more robust when somebody changes hgclient's
maxdatasize in the future.
Sean Farley - Jan. 11, 2017, 7:02 p.m.
Jun Wu <quark@fb.com> writes:

> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1484149164 -28800
> #      Wed Jan 11 23:39:24 2017 +0800
> # Node ID 5cbdc769201b22cbc4f698c7bc9235984c7cf427
> # Parent  e882c7bb5a0ba2589a44108c9a87b300a13e08df
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 5cbdc769201b
> chg: check snprintf result strictly
>
> This makes the program more robust when somebody changes hgclient's
> maxdatasize in the future.

This looks good to me.
Augie Fackler - Jan. 12, 2017, 3 a.m.
On Wed, Jan 11, 2017 at 11:42:57PM +0800, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1484149164 -28800
> #      Wed Jan 11 23:39:24 2017 +0800
> # Node ID 5cbdc769201b22cbc4f698c7bc9235984c7cf427
> # Parent  e882c7bb5a0ba2589a44108c9a87b300a13e08df
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 5cbdc769201b
> chg: check snprintf result strictly

Queued, thanks

>
> This makes the program more robust when somebody changes hgclient's
> maxdatasize in the future.
>
> diff --git a/contrib/chg/hgclient.c b/contrib/chg/hgclient.c
> --- a/contrib/chg/hgclient.c
> +++ b/contrib/chg/hgclient.c
> @@ -367,7 +367,9 @@ static void readhello(hgclient_t *hgc)
>  static void updateprocname(hgclient_t *hgc)
>  {
> -	size_t n = (size_t)snprintf(hgc->ctx.data, hgc->ctx.maxdatasize,
> +	int r = snprintf(hgc->ctx.data, hgc->ctx.maxdatasize,
>                       "chg[worker/%d]", (int)getpid());
> -	hgc->ctx.datasize = n;
> +	if (r < 0 || (size_t)r >= hgc->ctx.maxdatasize)
> +		abortmsg("insufficient buffer to write procname (r = %d)", r);
> +	hgc->ctx.datasize = (size_t)r;
>       writeblockrequest(hgc, "setprocname");
>  }
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/contrib/chg/hgclient.c b/contrib/chg/hgclient.c
--- a/contrib/chg/hgclient.c
+++ b/contrib/chg/hgclient.c
@@ -367,7 +367,9 @@  static void readhello(hgclient_t *hgc)
 static void updateprocname(hgclient_t *hgc)
 {
-	size_t n = (size_t)snprintf(hgc->ctx.data, hgc->ctx.maxdatasize,
+	int r = snprintf(hgc->ctx.data, hgc->ctx.maxdatasize,
 			"chg[worker/%d]", (int)getpid());
-	hgc->ctx.datasize = n;
+	if (r < 0 || (size_t)r >= hgc->ctx.maxdatasize)
+		abortmsg("insufficient buffer to write procname (r = %d)", r);
+	hgc->ctx.datasize = (size_t)r;
 	writeblockrequest(hgc, "setprocname");
 }