diff mbox

/proc vis rework

Message ID 20091211225835.GA17140@Linus-Debian (mailing list archive)
State RFC, archived
Headers

Commit Message

Linus Lüssing Dec. 11, 2009, 10:58 p.m. UTC 
  Hi Andrew,

your patch seems to work pretty well here, tested it with 9
batman-nodes in the same room. Nevertheless I found a couple of
smaller bugs in there:
- batctl segfaults, if "batctl vis" has no following argument
- batctl now displays the help-page in certain situations where we
  don't want it to
- batctl ommits the first TQ tupel
The attached patch for your patch should fix these issues :).

And I wanted to ask, what do you think about unifying the specific
help output? For instance having this "Usage: ..."-header and the
alignment for the following items the same way as it is done for
other batctl commands as well.

Cheers, Linus

PS:
Hmm, I'm also missing a couple of link/TQ entries already in the
unified /proc/../vis. The 9 nodes in the same room should be able
to see each other - the originator table on those nodes is also
saying so. But I think I had seen this before without your patch,
the problems has to be somewhere else. I'm attaching a batctl-vis-
and proc-vis-output of the setup here too, just in case someone
might spot some (more) parsing mistakes.
06:24:01:b7:69:c1,TQ 06:24:01:b7:6a:d9 247, TQ 06:24:01:b7:6a:d1 247, TQ 06:24:01:b7:61:1b 255, TQ 06:24:01:b7:6a:b3 255, TQ 06:24:01:b7:6a:fd 255, TQ 06:24:01:b7:61:49 251, HNA 72:43:e7:02:8f:96, HNA 00:24:01:b7:69:c1, PRIMARY, 
06:24:01:b7:61:19,TQ 06:24:01:b7:6a:d9 249, TQ 06:24:01:b7:6a:d1 255, TQ 06:24:01:b7:61:1b 255, TQ 06:24:01:b7:6a:b3 247, TQ 00:13:e8:50:c0:ff 248, HNA 00:24:01:b7:61:19, HNA d6:59:34:c6:ee:ce, PRIMARY, 
06:24:01:b7:6a:b3,TQ 06:24:01:b7:69:c1 251, TQ 06:24:01:b7:6a:d9 255, TQ 06:24:01:b7:6a:d1 254, TQ 06:24:01:b7:61:19 247, TQ 06:24:01:b7:61:1b 255, TQ 00:13:e8:50:c0:ff 247, TQ 06:24:01:b7:6a:fd 255, TQ 06:24:01:b7:61:49 255, HNA 6e:49:09:0e:30:1f, HNA 00:24:01:b7:6a:b3, PRIMARY, 
06:24:01:b7:6a:fd,TQ 06:24:01:b7:69:c1 255, TQ 06:24:01:b7:6a:d9 251, TQ 06:24:01:b7:61:19 247, TQ 06:24:01:b7:61:1b 255, TQ 06:24:01:b7:6a:b3 255, TQ 00:13:e8:50:c0:ff 251, TQ 06:24:01:b7:61:49 247, HNA be:38:d5:48:ad:58, HNA 00:24:01:b7:6a:fd, PRIMARY, 
06:24:01:b7:61:49,TQ 06:24:01:b7:69:c1 254, TQ 06:24:01:b7:6a:d1 251, TQ 06:24:01:b7:61:19 255, TQ 06:24:01:b7:61:1b 252, TQ 06:24:01:b7:6a:b3 255, TQ 00:13:e8:50:c0:ff 255, TQ 06:24:01:b7:6a:fd 255, HNA 00:24:01:b7:61:49, HNA 86:44:91:74:79:cc, PRIMARY, 
06:24:01:b7:6a:d9,TQ 06:24:01:b7:69:c1 255, TQ 06:24:01:b7:6a:d1 251, TQ 06:24:01:b7:61:19 255, TQ 06:24:01:b7:61:1b 251, TQ 06:24:01:b7:6a:b3 255, TQ 00:13:e8:50:c0:ff 255, TQ 06:24:01:b7:6a:fd 247, TQ 06:24:01:b7:61:49 250, HNA 00:24:01:b7:6a:d9, HNA 9a:6e:8f:f0:90:a2, PRIMARY, 
06:24:01:b7:6a:d1,TQ 06:24:01:b7:69:c1 251, TQ 06:24:01:b7:6a:d9 250, TQ 06:24:01:b7:61:19 255, TQ 06:24:01:b7:61:1b 255, TQ 06:24:01:b7:6a:b3 253, TQ 00:13:e8:50:c0:ff 251, TQ 06:24:01:b7:6a:fd 255, HNA 00:24:01:b7:6a:d1, HNA 9a:8e:ad:ea:e0:dc, PRIMARY, 
06:24:01:b7:61:1b,TQ 06:24:01:b7:6a:d9 255, TQ 06:24:01:b7:6a:d1 255, TQ 06:24:01:b7:61:19 247, TQ 06:24:01:b7:6a:b3 255, TQ 00:13:e8:50:c0:ff 255, TQ 06:24:01:b7:6a:fd 251, HNA 5e:2b:6b:67:d7:17, HNA 00:24:01:b7:61:1b, PRIMARY, 
00:13:e8:50:c0:ff,TQ 06:24:01:b7:69:c1 255, TQ 06:24:01:b7:6a:d9 251, TQ 06:24:01:b7:6a:d1 255, TQ 06:24:01:b7:61:1b 247, TQ 06:24:01:b7:6a:b3 255, TQ 06:24:01:b7:6a:fd 255, HNA 72:c5:18:83:3a:a3, PRIMARY,
digraph {
	"06:24:01:b7:69:c1" -> "06:24:01:b7:6a:d9" [label="1.32"]
	"06:24:01:b7:69:c1" -> "06:24:01:b7:6a:d1" [label="1.32"]
	"06:24:01:b7:69:c1" -> "06:24:01:b7:61:1b" [label="1.0"]
	"06:24:01:b7:69:c1" -> "06:24:01:b7:6a:b3" [label="1.0"]
	"06:24:01:b7:69:c1" -> "06:24:01:b7:6a:fd" [label="1.0"]
	"06:24:01:b7:69:c1" -> "06:24:01:b7:61:49" [label="1.15"]
	"06:24:01:b7:69:c1" -> "72:43:e7:02:8f:96" [label="HNA"]
	"06:24:01:b7:69:c1" -> "00:24:01:b7:69:c1" [label="HNA"]
	subgraph "cluster_06:24:01:b7:69:c1" {
		"06:24:01:b7:69:c1" [peripheries=2]
	}
	"06:24:01:b7:61:19" -> "06:24:01:b7:6a:d9" [label="1.24"]
	"06:24:01:b7:61:19" -> "06:24:01:b7:6a:d1" [label="1.0"]
	"06:24:01:b7:61:19" -> "06:24:01:b7:61:1b" [label="1.0"]
	"06:24:01:b7:61:19" -> "06:24:01:b7:6a:b3" [label="1.32"]
	"06:24:01:b7:61:19" -> "00:13:e8:50:c0:ff" [label="1.28"]
	"06:24:01:b7:61:19" -> "00:24:01:b7:61:19" [label="HNA"]
	"06:24:01:b7:61:19" -> "d6:59:34:c6:ee:ce" [label="HNA"]
	subgraph "cluster_06:24:01:b7:61:19" {
		"06:24:01:b7:61:19" [peripheries=2]
	}
	"06:24:01:b7:6a:b3" -> "06:24:01:b7:69:c1" [label="1.15"]
	"06:24:01:b7:6a:b3" -> "06:24:01:b7:6a:d9" [label="1.0"]
	"06:24:01:b7:6a:b3" -> "06:24:01:b7:6a:d1" [label="1.3"]
	"06:24:01:b7:6a:b3" -> "06:24:01:b7:61:19" [label="1.32"]
	"06:24:01:b7:6a:b3" -> "06:24:01:b7:61:1b" [label="1.0"]
	"06:24:01:b7:6a:b3" -> "00:13:e8:50:c0:ff" [label="1.32"]
	"06:24:01:b7:6a:b3" -> "06:24:01:b7:6a:fd" [label="1.0"]
	"06:24:01:b7:6a:b3" -> "06:24:01:b7:61:49" [label="1.0"]
	"06:24:01:b7:6a:b3" -> "6e:49:09:0e:30:1f" [label="HNA"]
	"06:24:01:b7:6a:b3" -> "00:24:01:b7:6a:b3" [label="HNA"]
	subgraph "cluster_06:24:01:b7:6a:b3" {
		"06:24:01:b7:6a:b3" [peripheries=2]
	}
	"06:24:01:b7:6a:fd" -> "06:24:01:b7:69:c1" [label="1.0"]
	"06:24:01:b7:6a:fd" -> "06:24:01:b7:6a:d9" [label="1.15"]
	"06:24:01:b7:6a:fd" -> "06:24:01:b7:61:19" [label="1.32"]
	"06:24:01:b7:6a:fd" -> "06:24:01:b7:61:1b" [label="1.0"]
	"06:24:01:b7:6a:fd" -> "06:24:01:b7:6a:b3" [label="1.0"]
	"06:24:01:b7:6a:fd" -> "00:13:e8:50:c0:ff" [label="1.15"]
	"06:24:01:b7:6a:fd" -> "06:24:01:b7:61:49" [label="1.32"]
	"06:24:01:b7:6a:fd" -> "be:38:d5:48:ad:58" [label="HNA"]
	"06:24:01:b7:6a:fd" -> "00:24:01:b7:6a:fd" [label="HNA"]
	subgraph "cluster_06:24:01:b7:6a:fd" {
		"06:24:01:b7:6a:fd" [peripheries=2]
	}
	"06:24:01:b7:61:49" -> "06:24:01:b7:69:c1" [label="1.3"]
	"06:24:01:b7:61:49" -> "06:24:01:b7:6a:d1" [label="1.15"]
	"06:24:01:b7:61:49" -> "06:24:01:b7:61:19" [label="1.0"]
	"06:24:01:b7:61:49" -> "06:24:01:b7:61:1b" [label="1.11"]
	"06:24:01:b7:61:49" -> "06:24:01:b7:6a:b3" [label="1.0"]
	"06:24:01:b7:61:49" -> "00:13:e8:50:c0:ff" [label="1.0"]
	"06:24:01:b7:61:49" -> "06:24:01:b7:6a:fd" [label="1.0"]
	"06:24:01:b7:61:49" -> "00:24:01:b7:61:49" [label="HNA"]
	"06:24:01:b7:61:49" -> "86:44:91:74:79:cc" [label="HNA"]
	subgraph "cluster_06:24:01:b7:61:49" {
		"06:24:01:b7:61:49" [peripheries=2]
	}
	"06:24:01:b7:6a:d9" -> "06:24:01:b7:69:c1" [label="1.0"]
	"06:24:01:b7:6a:d9" -> "06:24:01:b7:6a:d1" [label="1.15"]
	"06:24:01:b7:6a:d9" -> "06:24:01:b7:61:19" [label="1.0"]
	"06:24:01:b7:6a:d9" -> "06:24:01:b7:61:1b" [label="1.15"]
	"06:24:01:b7:6a:d9" -> "06:24:01:b7:6a:b3" [label="1.0"]
	"06:24:01:b7:6a:d9" -> "00:13:e8:50:c0:ff" [label="1.0"]
	"06:24:01:b7:6a:d9" -> "06:24:01:b7:6a:fd" [label="1.32"]
	"06:24:01:b7:6a:d9" -> "06:24:01:b7:61:49" [label="1.20"]
	"06:24:01:b7:6a:d9" -> "00:24:01:b7:6a:d9" [label="HNA"]
	"06:24:01:b7:6a:d9" -> "9a:6e:8f:f0:90:a2" [label="HNA"]
	subgraph "cluster_06:24:01:b7:6a:d9" {
		"06:24:01:b7:6a:d9" [peripheries=2]
	}
	"06:24:01:b7:6a:d1" -> "06:24:01:b7:69:c1" [label="1.15"]
	"06:24:01:b7:6a:d1" -> "06:24:01:b7:6a:d9" [label="1.20"]
	"06:24:01:b7:6a:d1" -> "06:24:01:b7:61:19" [label="1.0"]
	"06:24:01:b7:6a:d1" -> "06:24:01:b7:61:1b" [label="1.0"]
	"06:24:01:b7:6a:d1" -> "06:24:01:b7:6a:b3" [label="1.7"]
	"06:24:01:b7:6a:d1" -> "00:13:e8:50:c0:ff" [label="1.15"]
	"06:24:01:b7:6a:d1" -> "06:24:01:b7:6a:fd" [label="1.0"]
	"06:24:01:b7:6a:d1" -> "00:24:01:b7:6a:d1" [label="HNA"]
	"06:24:01:b7:6a:d1" -> "9a:8e:ad:ea:e0:dc" [label="HNA"]
	subgraph "cluster_06:24:01:b7:6a:d1" {
		"06:24:01:b7:6a:d1" [peripheries=2]
	}
	"06:24:01:b7:61:1b" -> "06:24:01:b7:6a:d9" [label="1.0"]
	"06:24:01:b7:61:1b" -> "06:24:01:b7:6a:d1" [label="1.0"]
	"06:24:01:b7:61:1b" -> "06:24:01:b7:61:19" [label="1.32"]
	"06:24:01:b7:61:1b" -> "06:24:01:b7:6a:b3" [label="1.0"]
	"06:24:01:b7:61:1b" -> "00:13:e8:50:c0:ff" [label="1.0"]
	"06:24:01:b7:61:1b" -> "06:24:01:b7:6a:fd" [label="1.15"]
	"06:24:01:b7:61:1b" -> "5e:2b:6b:67:d7:17" [label="HNA"]
	"06:24:01:b7:61:1b" -> "00:24:01:b7:61:1b" [label="HNA"]
	subgraph "cluster_06:24:01:b7:61:1b" {
		"06:24:01:b7:61:1b" [peripheries=2]
	}
	"00:13:e8:50:c0:ff" -> "06:24:01:b7:69:c1" [label="1.0"]
	"00:13:e8:50:c0:ff" -> "06:24:01:b7:6a:d9" [label="1.15"]
	"00:13:e8:50:c0:ff" -> "06:24:01:b7:6a:d1" [label="1.0"]
	"00:13:e8:50:c0:ff" -> "06:24:01:b7:61:1b" [label="1.32"]
	"00:13:e8:50:c0:ff" -> "06:24:01:b7:6a:b3" [label="1.0"]
	"00:13:e8:50:c0:ff" -> "06:24:01:b7:6a:fd" [label="1.0"]
	"00:13:e8:50:c0:ff" -> "72:c5:18:83:3a:a3" [label="HNA"]
	subgraph "cluster_00:13:e8:50:c0:ff" {
		"00:13:e8:50:c0:ff" [peripheries=2]
	}
}

Comments

Linus Lüssing Dec. 11, 2009, 11:20 p.m. UTC | #1 
Ah, sorry, and skip the functions.c-part please, I had submitted a
seperate patch for that here already.

Cheers, Linus

On Fri, Dec 11, 2009 at 11:58:35PM +0100, Linus Lüssing wrote:
> Hi Andrew,
> 
> your patch seems to work pretty well here, tested it with 9
> batman-nodes in the same room. Nevertheless I found a couple of
> smaller bugs in there:
> - batctl segfaults, if "batctl vis" has no following argument
> - batctl now displays the help-page in certain situations where we
>   don't want it to
> - batctl ommits the first TQ tupel
> The attached patch for your patch should fix these issues :).
> 
> And I wanted to ask, what do you think about unifying the specific
> help output? For instance having this "Usage: ..."-header and the
> alignment for the following items the same way as it is done for
> other batctl commands as well.
> 
> Cheers, Linus
> 
> PS:
> Hmm, I'm also missing a couple of link/TQ entries already in the
> unified /proc/../vis. The 9 nodes in the same room should be able
> to see each other - the originator table on those nodes is also
> saying so. But I think I had seen this before without your patch,
> the problems has to be somewhere else. I'm attaching a batctl-vis-
> and proc-vis-output of the setup here too, just in case someone
> might spot some (more) parsing mistakes.
Andrew Lunn Dec. 12, 2009, 10:43 a.m. UTC | #2 
On Fri, Dec 11, 2009 at 11:58:35PM +0100, Linus L??ssing wrote:
> Hi Andrew,
> 
> your patch seems to work pretty well here, tested it with 9
> batman-nodes in the same room. Nevertheless I found a couple of
> smaller bugs in there:
> - batctl segfaults, if "batctl vis" has no following argument
> - batctl now displays the help-page in certain situations where we
>   don't want it to
> - batctl ommits the first TQ tupel
> The attached patch for your patch should fix these issues :).

Thanks for the review. I will try to look at the details today or
tomorrow.

	Andrew
Andrew Lunn Dec. 13, 2009, 4:14 p.m. UTC | #3 
> And I wanted to ask, what do you think about unifying the specific
> help output? For instance having this "Usage: ..."-header and the
> alignment for the following items the same way as it is done for
> other batctl commands as well.

I thought about that. However the architecture allows different
formats to be easily added. Different formats might need different
optional arguments. It just seems easier the way it is at the moment.

	 Andrew
diff mbox

Patch

diff -ur batctl/functions.c batctl2/functions.c
--- batctl/functions.c	2009-12-11 23:10:31.000000000 +0100
+++ batctl2/functions.c	2009-12-11 23:09:48.000000000 +0100
@@ -161,10 +161,10 @@ 
 
 	if (read_opt & USE_READ_BUFF) {
 		read_ptr = read_buff;
-		read_len = sizeof(read_buff);
+		read_len = sizeof(read_buff)-1;
 	} else {
 		read_ptr = lbuff;
-		read_len = sizeof(lbuff);
+		read_len = sizeof(lbuff)-1;
 	}
 
 open:
diff -ur batctl/main.c batctl2/main.c
--- batctl/main.c	2009-12-11 23:10:31.000000000 +0100
+++ batctl2/main.c	2009-12-11 23:09:48.000000000 +0100
@@ -121,7 +121,7 @@ 
 
 		ret = handle_proc_setting(argc - 1, argv + 1, PROC_ORIG_INTERVAL, orig_interval_usage);
 
-	} if (strcmp(argv[1], "vis") == 0) {
+	} else if (strcmp(argv[1], "vis") == 0) {
 
 		ret = vis(argc - 1, argv + 1);
 
diff -ur batctl/vis.c batctl2/vis.c
--- batctl/vis.c	2009-12-11 23:10:48.000000000 +0100
+++ batctl2/vis.c	2009-12-11 23:29:04.000000000 +0100
@@ -215,7 +215,7 @@ 
   
   while ((read = getline(&line, &len, fp)) != -1) {
     /* First MAC address is the originator */
-    orig = strtok_r(line, " ", &line_save_ptr);
+    orig = strtok_r(line, ",", &line_save_ptr);
 
     duplet_save_ptr = line_save_ptr;
     while ((duplet = strtok_r(NULL, ",", &duplet_save_ptr)) != NULL) {
@@ -265,10 +265,12 @@ 
   int c;
   
   /* Do we know the requested format? */
-  if (strcmp(argv[1], "dot") == 0)
-    dot=true;
-  if (strcmp(argv[1], "json") == 0)
-    json=true;
+  if(argc > 1) {
+    if (strcmp(argv[1], "dot") == 0)
+      dot=true;
+    if (strcmp(argv[1], "json") == 0)
+      json=true;
+  }
   
   if (!dot && !json) {
     usage();
@@ -319,5 +321,3 @@ 
 
   return EXIT_FAILURE;
 }
-  
-