Re: [PATCH] ifupdown: make parser for /etc/network/interfaces more robust

[Peter Marschall <peter adpm de>]

Hi Dan,

On Monday, 9. August 2010, Dan Williams wrote:
> Hi,
> 
> Thanks for the patch!  Is there any chance you could provide some
> testcase /e/n/i files that are now expected to work with your rewrite,
> but which did not work before?  I'd like to start building up the
> testsuite for ifupdown.

Please find attached
a) my test file with lots of pathological cases 
    Renamed to etc_network_interfaces to not clobber my real /e/n/i
b) the patch I used to quickly show the effects of my parser changes.
    Hidden behind #ifdef's it converts interface_parser.c into a standalone
    program ;-)

Try the patch in b) with the old version and the new version, and you'll see 
the differences.

What I consider most important in the patch I sent on Sunday is the correct 
treatment of
* spaces/tabs as word-separators
  (I've seen wildly varying indentations, ...)
* multi-argument auto / allow-* lines
  (the man page of /e/n/i constains the example "auto lo eth0",
   so I guess multi-argument  auto / allow-* stanzas are used very often)

Best regards
Peter

-- 
Peter Marschall
peter adpm de

# etc_network_interfaces -- test file with testcases for NM's updated parser

# case 1: line before 1st block (must be ignored)
address 10.250.2.3

# case 2: wrapped line
auto \
lo 

# case 3: line wrapped over multiple lines & multi-argument allow-*
allow-hotplug eth0 \
		wlan0 \
		bnep0

# case 4: 'allow-auto' is synonymous to 'auto'
allow-auto eth0

# case 5: multi-argument allow-* (even worse: trailing space)
allow-hotplug eth0 wlan0 

# case 6: mix between tabs and spaces
   	iface  	 lo    inet	loopback  

# case 7: over-long line (must be ignored completely)
123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 

# case 8: over-long line that wraps to consecutive lines (must be ignored completely)
123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 \
allow-test	eth0 \
eth0

iface eth0 inet static
# case 9: wrapped lines inside a block (to be on the safe side)
	address \
		10.250.2.3
	netmask \
		255.255.255.192

	broadcast 10.250.2.63
	gateway 10.250.2.50
# case 10: again an over-long line (
123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 123456789 


iface pppoe inet manual
# case 11: wrapped line (without leading space on the wrapped part, wrap within a multi-word value)
	pre-up /sbin/ifconfig \
eth0 up
# case 12: wrapped line, splitting a word (must be joined again)
	up ifup ppp0\
=dsl

# case 13: variations of tabs & spaces
iface dsl	inet ppp 

# case 14: variations of tabs and spaces (all must be recognized as lines starting an iface block)
iface	wlan0 inet manual
 	iface wlan-adpm inet dhcp
iface   wlan-default   inet   dhcp

# case 15: trailing space (must be ignored)
iface bnep0 inet static 

# case 16: last line that is not followed by LF (added with 'echo -n "mapping eth0" >> /e/n/i')
mapping eth0

--- ifupdown/interface_parser.c	2010-08-10 17:49:38.987957067 +0200
+++ interface_parser.c	2010-08-10 17:51:02.359457371 +0200
@@ -25,18 +25,56 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include "nm-utils.h"
+
+#if !defined(INTERFACE_PARSER_TEST)
+# include "nm-utils.h"
+#else
+# undef		ENI_INTERFACES_FILE
+# define	ENI_INTERFACES_FILE	"etc_network_interfaces"
+
+# define	 nm_warning		printf
+#endif
 
 if_block* first;
 if_block* last;
 
 if_data* last_data;
 
+#if defined(INTERFACE_PARSER_TEST)
+int main(int argc, char *argv[])
+{
+	ifparser_init();
+
+	// dump blocks
+	if (first != NULL) {
+		if_block *n;
+		for (n = first; n != NULL; n = n->next) {
+			// each block start with its type & name 
+			// (single quotes used to show typ & name baoundaries)
+			printf("'%s' '%s'\n", n->type, n->name);
+			if_data *m;
+			// each key-value pair within a block is indented & separated by a tab
+			// (single quotes used to show typ & name baoundaries)
+			for (m = n->info; m != NULL; m = m->next) {
+				printf("\t'%s'\t'%s'\n", m->key, m->data);
+			}
+			// blocks are separated by an empty line
+			printf("\n");
+		}
+	}
+}
+#endif
+
 void add_block(const char *type, const char* name)
 {
 	if_block *ret = (if_block*)calloc(1,sizeof(struct _if_block));
+#if defined(INTERFACE_PARSER_TEST)
+	ret->name = strdup(name);
+	ret->type = strdup(type);
+#else
 	ret->name = g_strdup(name);
 	ret->type = g_strdup(type);
+#endif
 	if (first == NULL)
 		first = last = ret;
 	else
@@ -57,9 +95,13 @@
 		return;
 
 	ret = (if_data*) calloc(1,sizeof(struct _if_data));
+#if defined(INTERFACE_PARSER_TEST)
+	ret->key = strdup(key);
+	ret->data = strdup(data);
+#else
 	ret->key = g_strdup(key);
 	ret->data = g_strdup(data);
-
+#endif
 	if (last->info == NULL)
 	{
 		last->info = ret;