On 11/13/2010 04:52 PM, Eric Blake wrote:
* src/nwfilter/nwfilter_ebiptables_driver.c
(ebiptablesWriteToTempFile): Use /bin/sh.
(bash_cmd_path): Delete.
(ebiptablesDriverInit, ebiptablesDriverShutdown): No need to
search for bash.
(CMD_EXEC): Prefer $() over ``, since we can assume POSIX.
(iptablesSetupVirtInPost): Use portable 'test' syntax.
(iptablesLinkIPTablesBaseChain): Use POSIX $(()) syntax.
---
It turns out that the current use of test x == x in nwfilter
was technically safe after all, since we were requiring the
existence of bash (and that was also relatively reasonable,
since nwfilter is only compiled for Linux). But that still
feels awkward, when it is easier to just fix things to work
with /bin/sh. Here, I went the easy route, and assumed that
since things are Linux, we can use shell constructs that
aren't strictly portable to all /bin/sh implementations, so
long as they are portable to POSIX (and will thus work with
dash or busybox sh).
src/nwfilter/nwfilter_ebiptables_driver.c | 25 +++++++++++++------------
1 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 20d1ba3..c3a0d3e 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -52,12 +52,16 @@
#define CHAINPREFIX_HOST_IN_TEMP 'J'
#define CHAINPREFIX_HOST_OUT_TEMP 'P'
+/* This file generates a temporary shell script. Since ebiptables is
+ Linux-specific, we can be reasonably certain that /bin/sh is more
+ or less POSIX-compliant, so we can use $() and $(()). However, we
+ cannot assume that /bin/sh is bash, so stick to POSIX syntax. */
#define CMD_SEPARATOR "\n"
#define CMD_DEF_PRE "cmd='"
#define CMD_DEF_POST "'"
#define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST
-#define CMD_EXEC "eval res=\\`\"${cmd}\"\\`" CMD_SEPARATOR
+#define CMD_EXEC "eval res=\\$(\"${cmd}\")" CMD_SEPARATOR
#define CMD_STOPONERR(X) \
X ? "if [ $? -ne 0 ]; then" \
" echo \"Failure to execute command '${cmd}'.\";" \
@@ -76,7 +80,6 @@
static char *ebtables_cmd_path;
static char *iptables_cmd_path;
static char *ip6tables_cmd_path;
-static char *bash_cmd_path;
static char *grep_cmd_path;
static char *gawk_cmd_path;
@@ -427,7 +430,7 @@ static int iptablesLinkIPTablesBaseChain(const char *iptables_cmd,
" " CMD_DEF("%s -I %s %d -j %s") CMD_SEPARATOR
" " CMD_EXEC
" %s"
- " let r=r+1\n"
+ " r=$(( $r + 1 ))\n"
/bin/sh on my system points to bash. What else is it allowed to point
to? Obviously csh would be incompatible to /bin/sh.
" " CMD_DEF("%s -D %s ${r}") CMD_SEPARATOR
" " CMD_EXEC
" %s"
@@ -650,7 +653,7 @@ iptablesSetupVirtInPost(const char *iptables_cmd,
virBufferVSprintf(buf,
"res=$(%s -n -L " VIRT_IN_POST_CHAIN
" | grep \"\\%s %s\")\n"
- "if [ \"${res}\" == \"\" ]; then "
+ "if [ \"${res}\" = \"\" ]; then "
CMD_DEF("%s"
" -A " VIRT_IN_POST_CHAIN
" %s %s -j ACCEPT") CMD_SEPARATOR
@@ -2431,7 +2434,7 @@ ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED,
*
* Write the string into a temporary file and return the name of
* the temporary file. The string is assumed to contain executable
- * commands. A line '#!/bin/bash' will automatically be written
+ * commands. A line '#!/bin/sh' will automatically be written
* as the first line in the file. The permissions of the file are
* set so that the file can be run as an executable script.
*/
@@ -2444,7 +2447,7 @@ ebiptablesWriteToTempFile(const char *string) {
char *header;
size_t written;
- virBufferVSprintf(&buf, "#!%s\n", bash_cmd_path);
+ virBufferAddLit(&buf, "#!/bin/sh\n");
if (virBufferError(&buf)) {
virBufferFreeAndReset(&buf);
@@ -2513,10 +2516,10 @@ err_exit:
* commands executed via the script the was run.
*
* Returns 0 in case of success, != 0 in case of an error. The returned
- * value is NOT the result of running the commands inside the bash
+ * value is NOT the result of running the commands inside the shell
* script.
*
- * Execute a sequence of commands (held in the given buffer) as a bash
+ * Execute a sequence of commands (held in the given buffer) as a /bin/sh
* script and return the status of the execution.
*/
static int
@@ -3657,7 +3660,6 @@ ebiptablesDriverInit(void)
if (virMutexInit(&execCLIMutex))
return EINVAL;
- bash_cmd_path = virFindFileInPath("bash");
gawk_cmd_path = virFindFileInPath("gawk");
grep_cmd_path = virFindFileInPath("grep");
@@ -3701,9 +3703,9 @@ ebiptablesDriverInit(void)
VIR_FREE(ip6tables_cmd_path);
}
- /* ip(6)tables support needs bash, gawk& grep, ebtables doesn't */
+ /* ip(6)tables support needs gawk& grep, ebtables doesn't */
if ((iptables_cmd_path != NULL || ip6tables_cmd_path != NULL)&&
- (!grep_cmd_path || !bash_cmd_path || !gawk_cmd_path)) {
+ (!grep_cmd_path || !gawk_cmd_path)) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("essential tools to support ip(6)tables "
"firewalls could not be located"));
@@ -3730,7 +3732,6 @@ static void
ebiptablesDriverShutdown()
{
VIR_FREE(gawk_cmd_path);
- VIR_FREE(bash_cmd_path);
VIR_FREE(grep_cmd_path);
VIR_FREE(ebtables_cmd_path);
VIR_FREE(iptables_cmd_path);
Since you are the expert with shells and I trust that the TCK tests now
pass with all possible /bin/sh's, you get my ACK.
Stefan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list