From 6245d12224761a111d49f157683e1f023ae0e480 Mon Sep 17 00:00:00 2001
From: Aomx <aomx@riseup.net>
Date: Tue, 26 Jul 2016 15:39:24 +0200
Subject: [PATCH] adjustments regarding feedback

---
 .../net/packetdrill/packet_to_string_test.c   |  6 +--
 gtests/net/packetdrill/parser.y               | 13 +++----
 gtests/net/packetdrill/sctp_chunk_to_string.c |  4 +-
 gtests/net/packetdrill/sctp_packet.c          | 37 ++++++++++---------
 gtests/net/packetdrill/sctp_packet.h          |  5 +--
 5 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/gtests/net/packetdrill/packet_to_string_test.c b/gtests/net/packetdrill/packet_to_string_test.c
index a6324972..4774f7ed 100644
--- a/gtests/net/packetdrill/packet_to_string_test.c
+++ b/gtests/net/packetdrill/packet_to_string_test.c
@@ -346,7 +346,7 @@ static void test_sctp_ipv6_packet_to_string(void)
 		"CWR[flgs=0x00, tsn=16909060]; "
 		"SHUTDOWN_COMPLETE[flgs=T]; "
 		"FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{1,2},{3,4}]]; "
-		"I_FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{U,1,256}]]; "
+		"I_FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{1,1,256}]]; "
 		"I-DATA[flgs=IUBE, len=23, tsn=4, sid=255, mid=1, ppid=0]; "
 		"I-DATA[flgs=IUE, len=23, tsn=4, sid=255, mid=2, fsn=1]; "
 		"PAD[flgs=0x00, len=16, val=...]";
@@ -409,7 +409,7 @@ static void test_sctp_ipv6_packet_to_string(void)
 		"CWR[flgs=0x00, tsn=16909060]; "
 		"SHUTDOWN_COMPLETE[flgs=T]; "
 		"FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{1,2},{3,4}]]; "
-		"I_FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{U,1,256}]]; "
+		"I_FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{1,1,256}]]; "
 		"I-DATA[flgs=IUBE, len=23, tsn=4, sid=255, mid=1, ppid=0]; "
 		"I-DATA[flgs=IUE, len=23, tsn=4, sid=255, mid=2, fsn=1]; "
 		"PAD[flgs=0x00, len=16, val=...]";
@@ -472,7 +472,7 @@ static void test_sctp_ipv6_packet_to_string(void)
 		"CWR[flgs=0x00, tsn=16909060]; "
 		"SHUTDOWN_COMPLETE[flgs=T]; "
 		"FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{1,2},{3,4}]]; "
-		"I_FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{U,1,256}]]; "
+		"I_FORWARD_TSN[flgs=0x00, len=16, cum_tsn=3047862031, ids=[{1,1,256}]]; "
 		"I-DATA[flgs=IUBE, len=23, tsn=4, sid=255, mid=1, ppid=0]; "
 		"I-DATA[flgs=IUE, len=23, tsn=4, sid=255, mid=2, fsn=1]; "
 		"PAD[flgs=0x00, len=16, val=...]"
diff --git a/gtests/net/packetdrill/parser.y b/gtests/net/packetdrill/parser.y
index a2ad7f21..e0e4fdc5 100644
--- a/gtests/net/packetdrill/parser.y
+++ b/gtests/net/packetdrill/parser.y
@@ -1630,18 +1630,17 @@ i_forward_tsn_ids_list
 ;
 
 i_forward_tsn_id
-: '{' WORD ',' INTEGER ',' INTEGER '}' {
-	char *c = $2;
-	if (*c != 'O' && *c != 'U') {
-		semantic_error("either O for ordered or U for unordered must be specified");
-	}
-	if (!is_valid_u16($4)) {
+: '{' INTEGER ',' INTEGER ',' INTEGER '}' {
+	if (!is_valid_u16($2)) {
 		semantic_error("stream identifier out of range");
 	}
+        if (!is_valid_u16($4)) {
+		semantic_error("reserved out of range");
+	}
 	if (!is_valid_u32($6)) {
 		semantic_error("message identifier number out of range");
 	}
-	$$ = sctp_i_forward_tsn_ids_list_item_new(*c, $4, $6);
+	$$ = sctp_i_forward_tsn_ids_list_item_new($2, $4, $6);
 }
 ;
 
diff --git a/gtests/net/packetdrill/sctp_chunk_to_string.c b/gtests/net/packetdrill/sctp_chunk_to_string.c
index 6f3c6cd3..51b8c217 100644
--- a/gtests/net/packetdrill/sctp_chunk_to_string.c
+++ b/gtests/net/packetdrill/sctp_chunk_to_string.c
@@ -1768,9 +1768,9 @@ static int sctp_i_forward_tsn_chunk_to_string(
 	fprintf(s, "ids=[");
 	
 	for (i = 0; i < num_id_blocks; i++) {
-		fprintf(s, "{%s,%u,%u}",  
-			(ntohs(chunk->stream_identifier_blocks[i].reserved) & 1) ? "U" : "O",
+		fprintf(s, "{%u,%u,%u}",  
 			ntohs(chunk->stream_identifier_blocks[i].stream_identifier), 
+			ntohs(chunk->stream_identifier_blocks[i].reserved),
 			ntohl(chunk->stream_identifier_blocks[i].message_identifier));
 		if (i != num_id_blocks-1) {
 			fprintf(s, ",");
diff --git a/gtests/net/packetdrill/sctp_packet.c b/gtests/net/packetdrill/sctp_packet.c
index 866abba6..8be43c6e 100644
--- a/gtests/net/packetdrill/sctp_packet.c
+++ b/gtests/net/packetdrill/sctp_packet.c
@@ -341,14 +341,14 @@ void sctp_i_forward_tsn_ids_list_free (struct sctp_i_forward_tsn_ids_list *list)
 }
 
 struct sctp_i_forward_tsn_ids_list_item *
-sctp_i_forward_tsn_ids_list_item_new(char u_bit_set, u16 stream_identifier, u32 message_identifier) {
+sctp_i_forward_tsn_ids_list_item_new(u16 stream_identifier, u16 reserved, u32 message_identifier) {
 	struct sctp_i_forward_tsn_ids_list_item *item;
 
 	item = malloc(sizeof(struct sctp_i_forward_tsn_ids_list_item));
 	assert(item != NULL);
 	item->next = NULL;
 	item->stream_identifier = stream_identifier;
-	item->u_bit_set = u_bit_set;
+	item->reserved = reserved;
 	item->message_identifier = message_identifier;
 	return item;
 }
@@ -846,6 +846,8 @@ sctp_sack_chunk_new(s64 flgs, s64 cum_tsn, s64 a_rwnd,
 			chunk->block[i].gap.end = htons(item->block.gap.end);
 		}
 		assert((i == nr_gaps) && (item == NULL));
+		sctp_sack_block_list_free(gaps);
+		
 	}
 	if (dups != NULL) {
 		for (i = 0, item = dups->first;
@@ -854,6 +856,7 @@ sctp_sack_chunk_new(s64 flgs, s64 cum_tsn, s64 a_rwnd,
 			chunk->block[i + nr_gaps].tsn= htonl(item->block.tsn);
 		}
 		assert((i == nr_dups) && (item == NULL));
+		sctp_sack_block_list_free(dups);
 	}
 	return sctp_chunk_list_item_new((struct sctp_chunk *)chunk,
 	                                length, flags,
@@ -934,6 +937,7 @@ sctp_nr_sack_chunk_new(s64 flgs, s64 cum_tsn, s64 a_rwnd,
 			chunk->block[i].gap.end = htons(item->block.gap.end);
 		}
 		assert((i == nr_gaps) && (item == NULL));
+		sctp_sack_block_list_free(gaps);
 	}
 	if (nr_gaps_list != NULL) {
 		for (i = 0, item = nr_gaps_list->first;
@@ -943,6 +947,7 @@ sctp_nr_sack_chunk_new(s64 flgs, s64 cum_tsn, s64 a_rwnd,
 			chunk->block[i + nr_gaps].gap.end = htons(item->block.gap.end);
 		}
 		assert((i == number_of_nr_gaps) && (item == NULL));
+		sctp_sack_block_list_free(nr_gaps_list);
 	}
 	if (dups != NULL) {
 		for (i = 0, item = dups->first;
@@ -951,6 +956,7 @@ sctp_nr_sack_chunk_new(s64 flgs, s64 cum_tsn, s64 a_rwnd,
 			chunk->block[i + nr_gaps + number_of_nr_gaps].tsn= htonl(item->block.tsn);
 		}
 		assert((i == nr_dups) && (item == NULL));
+		sctp_sack_block_list_free(dups);
 	}
 	return sctp_chunk_list_item_new((struct sctp_chunk *)chunk,
 	                                length, flags,
@@ -1500,11 +1506,11 @@ sctp_pad_chunk_new(s64 flgs, s64 len, u8* padding)
 }
 
 struct sctp_chunk_list_item *
-sctp_forward_tsn_chunk_new(u32 cum_tsn, struct sctp_forward_tsn_ids_list *sids) {
+sctp_forward_tsn_chunk_new(u32 cum_tsn, struct sctp_forward_tsn_ids_list *ids_list) {
 	struct sctp_forward_tsn_chunk *chunk;
 	struct sctp_forward_tsn_ids_list_item *item;
 	
-	DEBUGP("sctp_forward_tsn_chunk_new called with cum_tsn = %d and sids_list = %p", cum_tsn, sids);
+	DEBUGP("sctp_forward_tsn_chunk_new called with cum_tsn = %d and sids_list = %p", cum_tsn, ids_list);
 	
 	u32 flags;
 	u32 length;
@@ -1512,12 +1518,12 @@ sctp_forward_tsn_chunk_new(u32 cum_tsn, struct sctp_forward_tsn_ids_list *sids)
 
 	flags = 0;
 	length = sizeof(struct sctp_forward_tsn_chunk);
-	if (sids == NULL) {
+	if (ids_list == NULL) {
 		nr_sids = 0;
 		flags |= FLAG_CHUNK_LENGTH_NOCHECK;
 		flags |= FLAG_FORWARD_TSN_CHUNK_IDS_NOCHECK;
 	} else {
-		nr_sids = sids->nr_entries;
+		nr_sids = ids_list->nr_entries;
 		length += nr_sids * sizeof(struct sctp_stream_identifier_block);
 	}
 	
@@ -1535,12 +1541,12 @@ sctp_forward_tsn_chunk_new(u32 cum_tsn, struct sctp_forward_tsn_ids_list *sids)
 		chunk->cum_tsn = htonl((u32)cum_tsn);
 	}
 	
-	if (nr_sids == 0 || sids == NULL) {
+	if (nr_sids == 0 || ids_list == NULL) {
 		flags |= FLAG_FORWARD_TSN_CHUNK_IDS_NOCHECK;
 	}
 
-	if (sids != NULL) {
-		for (i = 0, item = sids->first;
+	if (ids_list != NULL) {
+		for (i = 0, item = ids_list->first;
 		     (i < nr_sids) && (item != NULL);
 		     i++, item = item->next) {
 			chunk->stream_identifier_blocks[i].stream= htons(item->stream_identifier);
@@ -1548,7 +1554,9 @@ sctp_forward_tsn_chunk_new(u32 cum_tsn, struct sctp_forward_tsn_ids_list *sids)
 		}
 		
 		assert((i == nr_sids) && (item == NULL));
+		sctp_forward_tsn_ids_list_free(ids_list);
 	}
+	
 	return sctp_chunk_list_item_new((struct sctp_chunk *)chunk,
 	                                length, flags,
 	                                sctp_parameter_list_new(),
@@ -1600,19 +1608,14 @@ sctp_i_forward_tsn_chunk_new(u32 cum_tsn, struct sctp_i_forward_tsn_ids_list *id
 		     (i < nr_ids) && (item != NULL);
 		     i++, item = item->next) {
 			chunk->stream_identifier_blocks[i].stream_identifier= htons(item->stream_identifier);
-			if (item->u_bit_set == 'U') {
-				//TODO: htons required here?
-				chunk->stream_identifier_blocks[i].reserved = htons(0x01); 
-			}
-			else if (item->u_bit_set == 'O') {
-				chunk->stream_identifier_blocks[i].reserved = 0;
-			}
-			
+			chunk->stream_identifier_blocks[i].reserved = htons(item->reserved); 
 			chunk->stream_identifier_blocks[i].message_identifier = htonl(item->message_identifier);
 		}
 		
 		assert((i == nr_ids) && (item == NULL));
+		sctp_i_forward_tsn_ids_list_free(ids_list);
 	}
+	
 	return sctp_chunk_list_item_new((struct sctp_chunk *)chunk,
 	                                length, flags,
 	                                sctp_parameter_list_new(),
diff --git a/gtests/net/packetdrill/sctp_packet.h b/gtests/net/packetdrill/sctp_packet.h
index ea7ddece..6fece3a3 100644
--- a/gtests/net/packetdrill/sctp_packet.h
+++ b/gtests/net/packetdrill/sctp_packet.h
@@ -123,7 +123,6 @@ void
 sctp_forward_tsn_ids_list_append(struct sctp_forward_tsn_ids_list *list,
 			          struct sctp_forward_tsn_ids_list_item *item);
 
-// TODO: where to call this freeing method... sctp_sack_block_list_free and sctp_byte_list_free are unused...?
 void sctp_forward_tsn_ids_list_free (struct sctp_forward_tsn_ids_list *list);
 
 struct sctp_forward_tsn_ids_list_item *
@@ -132,8 +131,8 @@ sctp_forward_tsn_ids_list_item_new(u16 stream_identifier, u16 stream_sequence_nu
 
 struct sctp_i_forward_tsn_ids_list_item {
 	struct sctp_i_forward_tsn_ids_list_item *next;
-	char u_bit_set;
 	u16 stream_identifier;
+        u16 reserved;
 	u32 message_identifier;
 }; 
 
@@ -153,7 +152,7 @@ sctp_i_forward_tsn_ids_list_append(struct sctp_i_forward_tsn_ids_list *list,
 void sctp_i_forward_tsn_ids_list_free (struct sctp_i_forward_tsn_ids_list *list);
 
 struct sctp_i_forward_tsn_ids_list_item *
-sctp_i_forward_tsn_ids_list_item_new(char u_bit_set, u16 stream_identifier, u32 message_identifier);
+sctp_i_forward_tsn_ids_list_item_new(u16 stream_identifier, u16 reserved, u32 message_identifier);
 
 struct sctp_address_type_list_item {
 	struct sctp_address_type_list_item *next;
-- 
GitLab