[Openswan dev] fmt_common_shell_out warnings (fwd)

[Paul Wouters]

---------- Forwarded message ----------
Date: Mon, 25 Jan 2010 13:50:06 -0500 (EST)
From: D. Hugh Redelmeier <hugh at mimosa.com>
To: Paul Wouters <paul at xelerance.com>
Subject: Re: one more warning Q

| From: Paul Wouters <paul at xelerance.com>
|
| I'm not sure if this one makes sense either:
|
| diff --git a/programs/pluto/kernel.c b/programs/pluto/kernel.c
| index 66ffee8..c839ad5 100644
| --- a/programs/pluto/kernel.c
| +++ b/programs/pluto/kernel.c
| @@ -360,12 +360,12 @@ fmt_common_shell_out(char *buf, int blen, struct
| connection *c
|         char *p;
|         int   l;
|         strncat(srcip_str, "PLUTO_MY_SOURCEIP=", sizeof(srcip_str));
| -       strncat(srcip_str, "'", sizeof(srcip_str));
| +       strncat(srcip_str, "'", sizeof(srcip_str)-strlen(srcip_str)-1);
|         l = strlen(srcip_str);
|         p = srcip_str + l;
|
|         addrtot(&sr->this.host_srcip, 0, p, sizeof(srcip_str));
| -       strncat(srcip_str, "'", sizeof(srcip_str));
| +       strncat(srcip_str, "'", sizeof(srcip_str)-strlen(srcip_str)-1);

strncat is a stupidly designed function.  Whoever wrote these calls
assumed that strncat worked the way one would have expected.  These
changes are actually real bug fixes.

There remains one bug: the last arg to addrtot looks wrong to me.
Should be something like sizeof(srcip_str)-l.  BTW, l isn't 1.  It is
dumb to use l as a variable name.

how about [UNTESTED]:
 	{
 		char ib[ADDRTOT_BUF];

 		addrtot(&sr->this.host_srcip, 0, ib, sizeof(ib));
 		snprintf(srcip_str, sizeof(srcip_str), "PLUTO_MY_SOURCEIP='%s'", ib);
 	}
This is to replace the whole body of the if statement.  Isn't that
easier to understand?