Skip to contentSkip to conversation

Fix some bugs in hcidump.

PATCH In review James Courtier-Dutton 2 participants 6 messages Feb 23, 2004 Bluetooth drivers

lore
Share & tools
lore

James Courtier-Dutton

author
#0 lore raw

PATCH 1/1 Fix some bugs in hcidump.

Attached is a patch to apply to bluez-hcidump-1.5/parser/sdp.c

It fixes some of the printout.

Cheers
James

sdp.c renamed from sdp.c.org +7 -4
@@ -324,6 +324,7 @@
 	printf("pat");
 
 	if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {
+	        len = frm->len;
 		while (len - frm->len < n1 ) {
 			if (parse_de_hdr(frm,&n2) == SDP_DE_UUID) {
 				print_uuid(n2, frm);

@@ -351,7 +352,8 @@
 	printf("aid(s)");
 
 	if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {
-		while (len - frm->len <= n1 ) {
+                len = frm->len;
+		while (len - frm->len < n1 ) {
 			/* Print AttributeID */
 			if (parse_de_hdr(frm, &n2) == SDP_DE_UINT) {
 				switch(n2) {

@@ -367,13 +369,13 @@
 					break;
 				}
 			} else {
-				printf("\nERROR: Unexpected syntax\n");
+				printf("\nERROR: Unexpected syntax: (aid)\n");
 				raw_dump(level, frm);
 			}
 		}
 		printf("\n");
 	} else {
-		printf("\nERROR: Unexpected syntax\n");
+		printf("\nERROR: Unexpected syntax: (aid)\n");
 		raw_dump(level, frm);
 	}
 }

@@ -385,6 +387,7 @@
 	int   len = frm->len;
 
 	if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {
+	        len = frm->len;
 		while (len - frm->len < n1 ) {
 			/* Print AttributeID */
 			if ((parse_de_hdr(frm, &n2) == SDP_DE_UINT) &&

@@ -426,7 +429,7 @@
 	int   cnt = 0;
 
 	if (parse_de_hdr(frm, &n) == SDP_DE_SEQ) {
-	printf(" len 0x%x frm->len 0x%x n 0x%x\n", len, frm->len, n);
+	        len = frm->len;
 		while (len - frm->len < n ) {
 			p_indent(level, 0);
 			printf("srv rec #%d\n", cnt++);
maintainer
#1 lore raw

Hi James,

Quoted 3 lines

Attached is a patch to apply to bluez-hcidump-1.5/parser/sdp.c

It fixes some of the printout.

please always do the diff againts the CVS version.

sdp.c line 330

Quoted 10 lines
sdp.c renamed from sdp.c.org +1 -0
@@ -324,6 +324,7 @@
 	printf("pat");
 
 	if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {
+	        len = frm->len;
 		while (len - frm->len < n1 ) {
 			if (parse_de_hdr(frm,&n2) == SDP_DE_UUID) {
 				print_uuid(n2, frm);

Your change means this

		while (n1 > 0) {
			...
		}

If this is what you want then do it this way and remove unneeded variables. Give me a short description what this changes do and why you did this change.

Regards

Marcel

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
Bluez-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

author
#2 lore raw
Quoted 29 lines · Marcel Holtmann

Hi James,

Quoted 3 lines

Attached is a patch to apply to bluez-hcidump-1.5/parser/sdp.c

It fixes some of the printout.

please always do the diff againts the CVS version.

sdp.c line 330

Quoted 10 lines
sdp.c renamed from sdp.c.org +1 -0
@@ -324,6 +324,7 @@
 	printf("pat");
 
 	if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {
+	        len = frm->len;
 		while (len - frm->len < n1 ) {
 			if (parse_de_hdr(frm,&n2) == SDP_DE_UUID) {
 				print_uuid(n2, frm);

Your change means this

		while (n1 > 0) {
			...
		}
Quoted 9 lines

If this is what you want then do it this way and remove unneeded variables. Give me a short description what this changes do and why you did this change.

Regards

Marcel

while (len - frm->len < n1)
is NOT the same as
while (n1 > 0)

Each call to parse_de_hdr and print_uuid changes the value of frm->len. So, each time round the while loop (len - frm->len) becomes larger.

The n1 value is set by the previous line
"if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {"

This means we have n1 bytes of data we wish to parse inside the while loop.
So, we want to start the while loop with (len - frm->len) == 0
Each time round the loop (len - frm->len) will increase( due to calls to parse_de_hdr and print_uuid) , and the loop will exit when (len - frm->len) >= n1, which is what we want.

If we fail to set "len = frm->len;" just before the while loop, the previous line
"if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {"
changes frm->len, so the "int len = frm->len;" at the beginning of the function is no longer correct. I.E. len != frm->len after the "if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {" statement.

I hope this explanation is clear.

Do you want me to resubmit the patch as a "cvs -u diff" ?

Cheers
James

maintainer
#3 lore raw

Hi James,

Quoted 20 lines

Each call to parse_de_hdr and print_uuid changes the value of frm->len. So, each time round the while loop (len - frm->len) becomes larger.

The n1 value is set by the previous line
"if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {"

This means we have n1 bytes of data we wish to parse inside the while loop. So, we want to start the while loop with (len - frm->len) == 0 Each time round the loop (len - frm->len) will increase( due to calls to parse_de_hdr and print_uuid) , and the loop will exit when (len - frm->len) >= n1, which is what we want.

If we fail to set "len = frm->len;" just before the while loop, the previous line
"if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {"
changes frm->len, so the "int len = frm->len;" at the beginning of the function is no longer correct.
I.E. len != frm->len after the "if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {" statement.

Quoted 3 lines

I hope this explanation is clear.

Do you want me to resubmit the patch as a "cvs -u diff" ?

yes. And please remove the initial assignment of len.

Regards

Marcel

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
Bluez-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

author
#4 lore raw
Quoted 10 lines · Marcel Holtmann
Quoted 1 line

Do you want me to resubmit the patch as a "cvs -u diff" ?

yes. And please remove the initial assignment of len.

Regards

Marcel

See attached patch.
I have also added some extra checks so that the while loops will not go mad if there is a badly formed packet. I.E. n,n1,n2 get given bad values.

Cheers
James

Index: hcidump/parser/sdp.c
===================================================================
RCS file: /cvsroot/bluez/hcidump/parser/sdp.c,v
retrieving revision 1.22

parser/sdp.c +11 -9
diff -u -r1.22 sdp.c
@@ -279,7 +279,7 @@
 static inline void print_des(uint8_t de_type, int level, int n, struct frame *frm, int *split)
 {
 	int len = frm->len;
-	while (len - frm->len < n )
+	while ( (len - frm->len < n) && (frm->len > 0) ) 
 		print_de(level, frm, split);
 }
 

@@ -328,7 +328,7 @@
 
 static inline void print_srv_srch_pat(int level, struct frame *frm)
 {
-	int len = frm->len;
+	int len;
 	int n1;
 	int n2;
 

@@ -336,7 +336,8 @@
 	printf("pat");
 
 	if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {
-		while (len - frm->len <= n1 ) {
+	        len = frm->len;
+		while ( (len - frm->len < n1) && (frm->len > 0) ) {
 			if (parse_de_hdr(frm,&n2) == SDP_DE_UUID) {
 				print_uuid(n2, frm);
 			} else {

@@ -356,14 +357,15 @@
 {
 	uint16_t attr_id;
 	uint32_t attr_id_range;
-	int len = frm->len;
+	int len;
 	int n1, n2;
 
 	p_indent(level, frm);
 	printf("aid(s)");
 
 	if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {
-		while (len - frm->len <= n1 ) {
+	        len = frm->len;
+		while ( (len - frm->len < n1) && (frm->len > 0) ) {
 			/* Print AttributeID */
 			if (parse_de_hdr(frm, &n2) == SDP_DE_UINT) {
 				char *name;

@@ -398,10 +400,11 @@
 {
 	uint16_t attr_id;
 	int   n1, n2, split;
-	int   len = frm->len;
+	int   len;
 
 	if (parse_de_hdr(frm, &n1) == SDP_DE_SEQ) {
-		while (len - frm->len < n1 ) {
+	        len = frm->len;
+		while ( (len - frm->len < n1) && (frm->len > 0) ) {
 			/* Print AttributeID */
 			if (parse_de_hdr(frm, &n2) == SDP_DE_UINT && n2 == sizeof(attr_id)) {
 				char *name;

@@ -437,8 +440,7 @@
 	int   cnt = 0;
 
 	if (parse_de_hdr(frm, &n) == SDP_DE_SEQ) {
-//	printf(" len 0x%x frm->len 0x%x n 0x%x\n", len, frm->len, n);
-		while (len - frm->len < n ) {
+		while ( (len - frm->len < n) && (frm->len > 0) ) {
 			p_indent(level, 0);
 			printf("srv rec #%d\n", cnt++);
 			print_attr_list(level+1, frm);
maintainer
#5 lore raw

Hi James,

Quoted 2 lines

I have also added some extra checks so that the while loops will not go mad if there is a badly formed packet. I.E. n,n1,n2 get given bad values.

patch is applied with some cosmetical changes.

Regards

Marcel

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
Bluez-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel