Wireshark · Ethereal-dev: Re: [Ethereal-dev] Patch for packet-bacapp.c for bug 793 and a few as yet unlogged problems

[David Richards <d_m_richards@xxxxxxxxxxx>]

Sorry about that.  It did show up as a warning in the compile but I 
missed it in the ream of other stuff.  Here's (what for me is) a correct 
version with no warnings/errors.  Same as previous but with extraneous 
params removed.

Dave  Richards

Jaap Keuter wrote:

> Hi,
>
> I'm sorry but your patch seems broken:
>
> packet-bacapp.c: In function 'fAcknowledgeAlarmRequest':
> packet-bacapp.c:3517: error: too many arguments to function 'fTimeStamp'
> packet-bacapp.c:3525: error: too many arguments to function 'fTimeStamp'
> make[4]: *** [packet-bacapp.lo] Error 1
>
> Thanx,
> Jaap
>
> On Sat, 18 Mar 2006, David Richards wrote:
>
>  
>
> > Patch that fixes bug 793 and a few other things mostly revolving around
> > BACnet schedules.  Could someone please take care of this?
> >
> > 1) fDate - correctly handle wild card year
> > 2) fTime - rename local variables cut-and-pasted from fDate
> > 3) fCalendarEntry - do single-pass decoding rather than while loop since
> > the structure cannot be repeated.
> > 4) fDailySchedule - correctly handle enclosing context tags.
> > 5) fWeeklySchedule - correctly handle enclosing context tags
> > 6) fAcknowledgeAlarmRequest - fix function name spelling and tags 3 and
> > 5 are timeStamp, not time.
> > 7) fSpecialEvent - handle context tags correctly.
> > 8) fReadRangeRequest - Add cases from 2004 spec
> >
> >
> > Dave Richards
> >
> >    
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>
>
>  

Index: packet-bacapp.c
===================================================================
--- packet-bacapp.c	(revision 17561)
+++ packet-bacapp.c	(working copy)
@@ -1716,14 +1716,18 @@
 	proto_tree *subtree;
 
 	tag_len = fTagHeader (tvb, offset, &tag_no, &tag_info, &lvt);
-	year = tvb_get_guint8(tvb, offset+tag_len) + 1900;
+	year = tvb_get_guint8(tvb, offset+tag_len);
 	month = tvb_get_guint8(tvb, offset+tag_len+1);
 	day = tvb_get_guint8(tvb, offset+tag_len+2);
 	weekday = tvb_get_guint8(tvb, offset+tag_len+3);
 	if ((year == 255) && (day == 255) && (month == 255) && (weekday == 255))
+	{
 		ti = proto_tree_add_text(tree, tvb, offset, lvt+tag_len,
 			"%sany", label);
-	else
+	}
+	else if (year != 255)
+	{
+		year += 1900;
 		ti = proto_tree_add_text(tree, tvb, offset, lvt+tag_len,
 			"%s%s %d, %d, (Day of Week = %s)",
 			label, val_to_str(month,
@@ -1732,6 +1736,14 @@
 			day, year, val_to_str(weekday,
 				days,
 				"(%d) not found"));
+	}
+	else
+	{
+		ti = proto_tree_add_text(tree, tvb, offset, lvt+tag_len,
+			"%s%s %d, any year, (Day of Week = %s)",
+			label, val_to_str(month, months, "month (%d) not found"),
+			day, val_to_str(weekday, days, "(%d) not found"));
+	}
 	subtree = proto_item_add_subtree(ti, ett_bacapp_tag);
 	fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
 
@@ -1741,28 +1753,28 @@
 static guint
 fTime (tvbuff_t *tvb, proto_tree *tree, guint offset, const gchar *label)
 {
-	guint32 year, month, day, weekday, lvt;
+	guint32 hour, minute, second, msec, lvt;
 	guint8 tag_no, tag_info;
 	guint tag_len;
 	proto_item *ti;
 	proto_tree *subtree;
 
 	tag_len = fTagHeader (tvb, offset, &tag_no, &tag_info, &lvt);
-	year = tvb_get_guint8(tvb, offset+tag_len);
-	month = tvb_get_guint8(tvb, offset+tag_len+1);
-	day = tvb_get_guint8(tvb, offset+tag_len+2);
-	weekday = tvb_get_guint8(tvb, offset+tag_len+3);
-	if ((year == 255) && (day == 255) && (month == 255) && (weekday == 255))
+	hour = tvb_get_guint8(tvb, offset+tag_len);
+	minute = tvb_get_guint8(tvb, offset+tag_len+1);
+	second = tvb_get_guint8(tvb, offset+tag_len+2);
+	msec = tvb_get_guint8(tvb, offset+tag_len+3);
+	if ((hour == 255) && (minute == 255) && (second == 255) && (msec == 255))
 		ti = proto_tree_add_text(tree, tvb, offset,
 			lvt+tag_len, "%sany", label);
 	else
 		ti = proto_tree_add_text(tree, tvb, offset, lvt+tag_len,
 			"%s%d:%02d:%02d.%d %s = %02d:%02d:%02d.%d",
 			label,
-			year > 12 ? year -12 : year,
-			month, day, weekday,
-			year > 12 ? "P.M." : "A.M.",
-			year, month, day, weekday);
+			hour > 12 ? hour - 12 : hour,
+			minute, second, msec,
+			hour > 12 ? "P.M." : "A.M.",
+			hour, minute, second, msec);
 	subtree = proto_item_add_subtree(ti, ett_bacapp_tag);
 	fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
 	
@@ -1805,26 +1817,26 @@
 static guint
 fCalendaryEntry (tvbuff_t *tvb, proto_tree *tree, guint offset)
 {
-	guint lastoffset = 0;
+    guint8 tag_no, tag_info;
+    guint32 lvt;
 
-	while ((tvb_length_remaining(tvb, offset) > 0)&&(offset>lastoffset)) {  /* exit loop if nothing happens inside */
-		lastoffset = offset;
-	
-		switch (fTagNo(tvb, offset)) {
-		case 0:	/* Date */
-			offset = fDate    (tvb, tree, offset, "Date: ");
-			break;
-		case 1:	/* dateRange */
-			offset = fDateRange (tvb, tree, offset);
-			break;
-		case 2:	/* BACnetWeekNDay */
-			offset = fWeekNDay (tvb, tree, offset);
-			break;
-		default:
-			return offset;
-			break;
-		}
+	switch (fTagNo(tvb, offset)) {
+	case 0:	/* Date */
+		offset = fDate    (tvb, tree, offset, "Date: ");
+		break;
+	case 1:	/* dateRange */
+        offset += fTagHeaderTree(tvb, tree, offset, &tag_no, &tag_info, &lvt);
+		offset = fDateRange (tvb, tree, offset);
+        offset += fTagHeaderTree(tvb, tree, offset, &tag_no, &tag_info, &lvt);
+		break;
+	case 2:	/* BACnetWeekNDay */
+		offset = fWeekNDay (tvb, tree, offset);
+		break;
+	default:
+		return offset;
+		break;
 	}
+
 	return offset;
 }
 
@@ -2623,28 +2635,27 @@
 	guint8 tag_no, tag_info;
 	guint32 lvt;
 	
-	while ((tvb_length_remaining(tvb, offset) > 0)&&(offset>lastoffset)) {  /* exit loop if nothing happens inside */
-		lastoffset = offset;
-		fTagHeader (tvb, offset, &tag_no, &tag_info, &lvt);
-		if (tag_is_closing(tag_info)) {
-			offset += fTagHeaderTree (tvb, subtree, offset,
-				&tag_no, &tag_info, &lvt);
-			return offset;
-		}
-		
-		switch (tag_no) {
-		case 0: /* day-schedule */
-			if (tag_is_opening(tag_info)) {
-				offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
-				offset = fTimeValue (tvb, subtree, offset);
-				break;
+	fTagHeader (tvb, offset, &tag_no, &tag_info, &lvt);
+	if (tag_is_opening(tag_info) && tag_no == 0)
+	{
+		offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt); /* opening context tag 0 */
+		while ((tvb_length_remaining(tvb, offset) > 0)&&(offset>lastoffset)) {  /* exit loop if nothing happens inside */
+			lastoffset = offset;
+			fTagHeader (tvb, offset, &tag_no, &tag_info, &lvt);
+			if (tag_is_closing(tag_info)) {
+				/* should be closing context tag 0 */
+				offset += fTagHeaderTree (tvb, subtree, offset,	&tag_no, &tag_info, &lvt);
+				return offset;
 			}
-			FAULT;
-			break;
-		default:
-			return offset;
+			
+			offset = fTimeValue (tvb, subtree, offset);
 		}
 	}
+	else if (tag_no == 0 && lvt == 0)
+	{
+		/* not sure null (empty array element) is legal */
+		offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
+	}
 	return offset;
 }
 
@@ -2662,9 +2673,7 @@
 		lastoffset = offset;
 		fTagHeader (tvb, offset, &tag_no, &tag_info, &lvt);
 		if (tag_is_closing(tag_info)) {
-			offset += fTagHeaderTree (tvb, tree, offset,
-				&tag_no, &tag_info, &lvt);
-			return offset;
+			return offset; /* outer encoding will print out closing tag */
 		}
 		tt = proto_tree_add_text(tree, tvb, offset, 0, val_to_str(i++, days, "day of week (%d) not found"));
 		subtree = proto_item_add_subtree(tt, ett_bacapp_value);
@@ -3484,9 +3493,11 @@
 }
 
 static guint
-fAcknowlegdeAlarmRequest (tvbuff_t *tvb, proto_tree *tree, guint offset)
+fAcknowledgeAlarmRequest (tvbuff_t *tvb, proto_tree *tree, guint offset)
 {
 	guint lastoffset = 0;
+	guint8 tag_no = 0, tag_info = 0;
+	guint32 lvt = 0;
 
 	while ((tvb_length_remaining(tvb, offset) > 0)&&(offset>lastoffset)) {  /* exit loop if nothing happens inside */
 		lastoffset = offset;
@@ -3502,13 +3513,17 @@
 				"event State Acknowledged: ", BACnetEventState, 64);
 			break;
 		case 3:	/* timeStamp */
-			offset = fTime (tvb, tree, offset, "time Stamp: ");
+			offset += fTagHeaderTree(tvb, tree, offset, &tag_no, &tag_info, &lvt);
+			offset = fTimeStamp(tvb, tree, offset);
+			offset += fTagHeaderTree(tvb, tree, offset, &tag_no, &tag_info, &lvt);
 			break;
 		case 4:	/* acknowledgementSource */
 			offset = fCharacterString (tvb, tree, offset, "acknowledgement Source: ");
 			break;
 		case 5:	/* timeOfAcknowledgement */
-			offset = fTime (tvb, tree, offset, "time Of Acknowledgement: ");
+			offset += fTagHeaderTree(tvb, tree, offset, &tag_no, &tag_info, &lvt);
+			offset = fTimeStamp(tvb, tree, offset);
+			offset += fTagHeaderTree(tvb, tree, offset, &tag_no, &tag_info, &lvt);
 			break;
 		default:
 			return offset;
@@ -4220,21 +4235,26 @@
 		lastoffset = offset;
 		fTagHeader (tvb, offset, &tag_no, &tag_info, &lvt);
 		if (tag_is_closing(tag_info)) {   
-			offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
 			continue;
 		}
         
-		switch (fTagNo(tvb,offset)) {
+		switch (tag_no) {
 		case 0:	/* calendaryEntry */
-			offset = fCalendaryEntry (tvb, subtree, offset);
+            if (tag_is_opening(tag_info))
+            {
+				offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
+    			offset = fCalendaryEntry (tvb, subtree, offset);
+				offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
+            }
 			break;
 		case 1:	/* calendarReference */
 			offset = fObjectIdentifier (tvb, subtree, offset);
 			break;
-		case 2:	/* calendarReference */
+		case 2:	/* list of BACnetTimeValue */
 			if (tag_is_opening(tag_info)) {
 				offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
 				offset = fTimeValue (tvb, subtree, offset);
+				offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
 				break;
 			}
 			FAULT;
@@ -4550,11 +4570,12 @@
 			FAULT;
 			break;
 		case 4:	/* range byTime */
+        case 7: /* 2004 spec */
 			if (tag_is_opening(tag_info)) {
 				tt = proto_tree_add_text(subtree, tvb, offset, 1, "range byTime");
 				subtree = proto_item_add_subtree(tt, ett_bacapp_value);
 				offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
-				offset = fApplicationTypes (tvb, subtree, offset, "reference Time: ");
+				offset = fDateTime(tvb, subtree, offset, "reference Date/Time: ");
 				offset = fApplicationTypes (tvb, subtree, offset, "reference Count: ");
 				break;
 			}
@@ -4571,6 +4592,17 @@
 			}
 			FAULT;
 			break;
+        case 6: /* range bySequenceNumber, 2004 spec */
+            if (tag_is_opening(tag_info)) {
+				tt = proto_tree_add_text(subtree, tvb, offset, 1, "range bySequenceNumber");
+				subtree = proto_item_add_subtree(tt, ett_bacapp_value);
+				offset += fTagHeaderTree (tvb, subtree, offset, &tag_no, &tag_info, &lvt);
+				offset = fApplicationTypes (tvb, subtree, offset, "referenceIndex: ");
+				offset = fApplicationTypes (tvb, subtree, offset, "reference Count: ");
+				break;
+            }
+			FAULT;
+			break;
 		default:
 			return offset;
 		}
@@ -4794,7 +4826,7 @@
 
 	switch (service_choice) {
 	case 0:	/* acknowledgeAlarm */
-		offset = fAcknowlegdeAlarmRequest (tvb, tree, offset);
+		offset = fAcknowledgeAlarmRequest (tvb, tree, offset);
 		break;
 	case 1: /* confirmedCOVNotification */
 		offset = fConfirmedCOVNotificationRequest (tvb, tree, offset);