[Openswan dev] patch for bug 1067
D. Hugh Redelmeier
hugh at mimosa.com
Sun Oct 25 15:09:22 EDT 2009
A patch for this bug report has been adopted.
https://gsoc.xelerance.com/issues/1067
MCR mentioned that there appeared to be problems with it so I had a
look.
Systems without popen(3) are broken. So this workaround does not deserve
to be first class. Perhaps #ifdef NO_POPEN.
Worse, apparently there is a popen, but it always returns failure.
That is crazy! So crazy that it deserves investigation (but not by
me).
Checking errno without first checking if there had been an error (i.e.
f == NULL) is wrong. Thus this patch introduces a bug into all
versions of Openswan, not just those without a working popen.
system(3) is not a work-alike for popen. You cannot just do one in
place of the other. The diagnostics produced by the command matter.
The returned result of the system(3) call is ignored. That's bad. It
should be logged if it indicates an error.
The signal handler for SIGCHLD is not restored if the system(3) path
is used.
Unconditionally returning TRUE is wrong.
================
While I'm looking at this code, I might as well ask about some lines
early in this function. Feel free to ignore: no bugs are disclosed.
DBG(DBG_CONTROL, DBG_log("executing %s%s: %s"
, verb, verb_suffix, cmd));
{
char tmp[100];
int slen,i;
memset(tmp,0,sizeof(tmp));
slen=strlen(cmd);
DBG(DBG_CONTROL, DBG_log("popen(): cmd is %d chars long", slen));
for(i=0; i<slen; i+=80) {
strncpy(tmp,&cmd[i],80);
DBG(DBG_CONTROL, DBG_log("cmd(%4d):%s:", i, tmp));
}
}
What's the idea of the braced section? What does it do that the first
command does not? Do commands ever get longer than 80 characters?
Here's a simpler version of the braced code, just in case there is
some point to this. UNTESTED!
DBG(DBG_CONTROL, {
int slen = strlen(cmd);
int i;
DBG_log("popen(): cmd is %d chars long", slen);
for(i=0; i<slen; i+=80)
DBG_log("cmd(%4d):%.80s:", i, cmd+i);
});
More information about the Dev
mailing list