bonefish + axeld:

* There was one incorrect check in BufferQueue::Get() that happened because
  the wrong operator was used due to the uint32 cast operator.
* Consequently, we removed the uint32 cast operator, and changed the code
  to deal with this. Fortunately, no other bugs were observed.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@29928 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/network/protocols/tcp/BufferQueue.cpp

@@ -91,14 +91,15 @@ BufferQueue::Add(net_buffer *buffer, tcp_sequence sequence)
 	}
 	if (sequence < fFirstSequence) {
 		// this buffer contains data that is already long gone - trim it
-		gBufferModule->remove_header(buffer, fFirstSequence - sequence);
+		gBufferModule->remove_header(buffer,
+			(fFirstSequence - sequence).Number());
 		sequence = fFirstSequence;
 	}
 
 	if (fList.IsEmpty() || sequence >= fLastSequence) {
 		// we usually just add the buffer to the end of the queue
 		fList.Add(buffer);
-		buffer->sequence = sequence;
+		buffer->sequence = sequence.Number();
 
 		if (sequence == fLastSequence
 			&& fLastSequence - fFirstSequence == fNumBytes) {
@@ -157,7 +158,7 @@ BufferQueue::Add(net_buffer *buffer, tcp_sequence sequence)
 				> sequence) {
 			// We already have the first part of this buffer
 			gBufferModule->remove_header(buffer,
-				previous->sequence + previous->size - sequence);
+				(previous->sequence + previous->size - sequence).Number());
 			sequence = previous->sequence + previous->size;
 		}
 	}
@@ -179,7 +180,7 @@ BufferQueue::Add(net_buffer *buffer, tcp_sequence sequence)
 		} else if (tcp_sequence(next->sequence) > sequence) {
 			// We have the end of this buffer already
 			gBufferModule->remove_trailer(buffer,
-				sequence + buffer->size - next->sequence);
+				(sequence + buffer->size - next->sequence).Number());
 		} else {
 			// We already have this data
 			gBufferModule->free(buffer);
@@ -196,7 +197,7 @@ BufferQueue::Add(net_buffer *buffer, tcp_sequence sequence)
 	}
 
 	fList.Insert(next, buffer);
-	buffer->sequence = sequence;
+	buffer->sequence = sequence.Number();
 	fNumBytes += buffer->size;
 
 	// we might need to update the number of bytes available
@@ -253,7 +254,7 @@ BufferQueue::RemoveUntil(tcp_sequence sequence)
 			gBufferModule->free(buffer);
 		} else {
 			// remove the header as far as needed
-			size_t size = sequence - buffer->sequence;
+			size_t size = (sequence - buffer->sequence).Number();
 			gBufferModule->remove_header(buffer, size);
 
 			buffer->sequence += size;
@@ -290,7 +291,7 @@ BufferQueue::Get(net_buffer *buffer, tcp_sequence sequence, size_t bytes)
 		return B_BAD_VALUE;
 	}
 	if (tcp_sequence(sequence + bytes) > fLastSequence)
-		bytes = fLastSequence - sequence;
+		bytes = (fLastSequence - sequence).Number();
 
 	size_t bytesLeft = bytes;
 
@@ -305,14 +306,14 @@ BufferQueue::Get(net_buffer *buffer, tcp_sequence sequence, size_t bytes)
 
 	if (source == NULL)
 		panic("we should have had that data...");
-	if (source->sequence > sequence) {
+	if (tcp_sequence(source->sequence) > sequence) {
 		panic("source %p, sequence = %lu (%lu)\n", source, source->sequence,
-			(uint32)sequence);
+			sequence.Number());
 	}
 
 	// clone the data
 
-	uint32 offset = sequence - source->sequence;
+	uint32 offset = (sequence - source->sequence).Number();
 
 	while (source != NULL && bytesLeft > 0) {
 		size_t size = min_c(source->size - offset, bytesLeft);
@@ -416,10 +417,10 @@ BufferQueue::Get(size_t bytes, bool remove, net_buffer **_buffer)
 size_t
 BufferQueue::Available(tcp_sequence sequence) const
 {
-	if (sequence > (uint32)fFirstSequence + fContiguousBytes)
+	if (sequence > (fFirstSequence + fContiguousBytes).Number())
 		return 0;
 
-	return fContiguousBytes + fFirstSequence - sequence;
+	return (fContiguousBytes + fFirstSequence - sequence).Number();
 }
 
 

src/add-ons/kernel/network/protocols/tcp/BufferQueue.h

@@ -76,8 +76,8 @@ BufferQueue::PushedData() const
 {
 	// we must check if fPushPointer is not 0 here due to
 	// tcp_sequence's special handling of >
-	return fPushPointer != 0
+	return fPushPointer != 0 && fPushPointer > fFirstSequence
-		&& fPushPointer > fFirstSequence ? fPushPointer - fFirstSequence : 0;
+		? (fPushPointer - fFirstSequence).Number() : 0;
 }
 
 #endif	// BUFFER_QUEUE_H

src/add-ons/kernel/network/protocols/tcp/TCPEndpoint.cpp

@@ -1548,7 +1548,7 @@ TCPEndpoint::_Receive(tcp_segment_header& segment, net_buffer* buffer)
 		// the window must not shrink
 
 	// trim buffer to be within the receive window
-	int32 drop = fReceiveNext - segment.sequence;
+	int32 drop = (int32)(fReceiveNext - segment.sequence).Number();
 	if (drop > 0) {
 		if ((uint32)drop > buffer->size
 			|| ((uint32)drop == buffer->size
@@ -1566,7 +1566,8 @@ TCPEndpoint::_Receive(tcp_segment_header& segment, net_buffer* buffer)
 
 	int32 action = KEEP;
 
-	drop = segment.sequence + buffer->size - (fReceiveNext + fReceiveWindow);
+	drop = (int32)(segment.sequence + buffer->size
+		- (fReceiveNext + fReceiveWindow)).Number();
 	if (drop > 0) {
 		// remove data exceeding our window
 		if ((uint32)drop >= buffer->size) {
@@ -1844,7 +1845,7 @@ TCPEndpoint::_ShouldSendSegment(tcp_segment_header& segment, uint32 length,
 	if (segment.advertised_window > 0) {
 		// correct the window to take into account what already has been advertised
 		uint32 window = (segment.advertised_window << fReceiveWindowShift)
-			- (fReceiveMaxAdvertised - fReceiveNext);
+			- (fReceiveMaxAdvertised - fReceiveNext).Number();
 
 		// if we can advertise a window larger than twice the maximum segment
 		// size, or half the maximum buffer size we send a window update
@@ -1912,14 +1913,14 @@ TCPEndpoint::_SendQueued(bool force, uint32 sendWindow)
 	else
 		segment.advertised_window = min_c(TCP_MAX_WINDOW, availableBytes);
 
-	segment.acknowledge = fReceiveNext;
+	segment.acknowledge = fReceiveNext.Number();
 
 	// Process urgent data
 	if (fSendUrgentOffset > fSendNext) {
 		segment.flags |= TCP_FLAG_URGENT;
-		segment.urgent_offset = fSendUrgentOffset - fSendNext;
+		segment.urgent_offset = (fSendUrgentOffset - fSendNext).Number();
 	} else {
-		fSendUrgentOffset = fSendUnacknowledged;
+		fSendUrgentOffset = fSendUnacknowledged.Number();
 			// Keep urgent offset updated, so that it doesn't reach into our
 			// send window on overlap
 		segment.urgent_offset = 0;
@@ -1942,8 +1943,8 @@ TCPEndpoint::_SendQueued(bool force, uint32 sendWindow)
 	// reduced (by congestion for instance), so at some point in time flight
 	// size may be larger than the currently calculated window.
 
-	uint32 flightSize = fSendMax - fSendUnacknowledged;
+	uint32 flightSize = (fSendMax - fSendUnacknowledged).Number();
-	uint32 consumedWindow = fSendNext - fSendUnacknowledged;
+	uint32 consumedWindow = (fSendNext - fSendUnacknowledged).Number();
 
 	if (consumedWindow > sendWindow) {
 		sendWindow = 0;
@@ -1998,7 +1999,7 @@ TCPEndpoint::_SendQueued(bool force, uint32 sendWindow)
 		PeerAddress().CopyTo(buffer->destination);
 
 		uint32 size = buffer->size;
-		segment.sequence = fSendNext;
+		segment.sequence = fSendNext.Number();
 
 		TRACE("SendQueued(): buffer %p (%lu bytes) address %s to %s\n"
 			"\tflags 0x%x, seq %lu, ack %lu, rwnd %hu, cwnd %lu, ssthresh %lu\n"
@@ -2033,7 +2034,7 @@ TCPEndpoint::_SendQueued(bool force, uint32 sendWindow)
 		if (segment.flags & TCP_FLAG_FINISH)
 			size++;
 
-		uint32 sendMax = fSendMax;
+		uint32 sendMax = fSendMax.Number();
 		fSendNext += size;
 		if (fSendMax < fSendNext)
 			fSendMax = fSendNext;
@@ -2205,7 +2206,7 @@ TCPEndpoint::_UpdateRoundTripTime(int32 roundTripTime)
 void
 TCPEndpoint::_ResetSlowStart()
 {
-	fSlowStartThreshold = max_c((fSendMax - fSendUnacknowledged) / 2,
+	fSlowStartThreshold = max_c((fSendMax - fSendUnacknowledged).Number() / 2,
 		2 * fSendMaxSegmentSize);
 	fCongestionWindow = fSendMaxSegmentSize;
 }
@@ -2300,11 +2301,11 @@ TCPEndpoint::Dump() const
 	kprintf("  accept sem: %ld\n", fAcceptSemaphore);
 	kprintf("  options: 0x%lx\n", (uint32)fOptions);
 	kprintf("  send\n");
-	kprintf("    window shift: %lu\n", (uint32)fSendWindowShift);
+	kprintf("    window shift: %u\n", fSendWindowShift);
-	kprintf("    unacknowledged: %lu\n", (uint32)fSendUnacknowledged);
+	kprintf("    unacknowledged: %lu\n", fSendUnacknowledged.Number());
-	kprintf("    next: %lu\n", (uint32)fSendNext);
+	kprintf("    next: %lu\n", fSendNext.Number());
-	kprintf("    max: %lu\n", (uint32)fSendMax);
+	kprintf("    max: %lu\n", fSendMax.Number());
-	kprintf("    urgent offset: %lu\n", (uint32)fSendUrgentOffset);
+	kprintf("    urgent offset: %lu\n", fSendUrgentOffset.Number());
 	kprintf("    window: %lu\n", fSendWindow);
 	kprintf("    max window: %lu\n", fSendMaxWindow);
 	kprintf("    max segment size: %lu\n", fSendMaxSegmentSize);
@@ -2312,25 +2313,25 @@ TCPEndpoint::Dump() const
 #if DEBUG_BUFFER_QUEUE
 	fSendQueue.Dump();
 #endif
-	kprintf("    last acknowledge sent: %lu\n", (uint32)fLastAcknowledgeSent);
+	kprintf("    last acknowledge sent: %lu\n", fLastAcknowledgeSent.Number());
-	kprintf("    initial sequence: %lu\n", (uint32)fInitialSendSequence);
+	kprintf("    initial sequence: %lu\n", fInitialSendSequence.Number());
 	kprintf("  receive\n");
-	kprintf("    window shift: %lu\n", (uint32)fReceiveWindowShift);
+	kprintf("    window shift: %u\n", fReceiveWindowShift);
-	kprintf("    next: %lu\n", (uint32)fReceiveNext);
+	kprintf("    next: %lu\n", fReceiveNext.Number());
-	kprintf("    max advertised: %lu\n", (uint32)fReceiveMaxAdvertised);
+	kprintf("    max advertised: %lu\n", fReceiveMaxAdvertised.Number());
-	kprintf("    window: %lu\n", (uint32)fReceiveWindow);
+	kprintf("    window: %lu\n", fReceiveWindow);
-	kprintf("    max segment size: %lu\n", (uint32)fReceiveMaxSegmentSize);
+	kprintf("    max segment size: %lu\n", fReceiveMaxSegmentSize);
 	kprintf("    queue: %lu / %lu\n", fReceiveQueue.Available(),
 		fReceiveQueue.Size());
 #if DEBUG_BUFFER_QUEUE
 	fReceiveQueue.Dump();
 #endif
-	kprintf("    initial sequence: %lu\n", (uint32)fInitialReceiveSequence);
+	kprintf("    initial sequence: %lu\n", fInitialReceiveSequence.Number());
 	kprintf("    duplicate acknowledge count: %lu\n",
 		fDuplicateAcknowledgeCount);
 	kprintf("  round trip time: %ld (deviation %ld)\n", fRoundTripTime,
 		fRoundTripDeviation);
-	kprintf("  retransmit timeout: %llu\n", (uint64)fRetransmitTimeout);
+	kprintf("  retransmit timeout: %lld\n", fRetransmitTimeout);
 	kprintf("  congestion window: %lu\n", fCongestionWindow);
 	kprintf("  slow start threshold: %lu\n", fSlowStartThreshold);
 }

src/add-ons/kernel/network/protocols/tcp/tcp.h

@@ -73,29 +73,106 @@ struct tcp_header {
 
 class tcp_sequence {
 public:
-					tcp_sequence() {}
+	inline tcp_sequence() {}
-					tcp_sequence(uint32 sequence) : fNumber(sequence) {}
+	inline tcp_sequence(uint32 sequence)
+		: fNumber(sequence)
+	{
+	}
 
-					operator uint32() const { return fNumber; }
+	inline uint32 Number() const
+	{
+		return fNumber;
+	}
 
-			void	operator=(uint32 sequence) { fNumber = sequence; }
+	inline tcp_sequence& operator=(tcp_sequence sequence)
-			bool	operator>(uint32 sequence) const
+	{
-						{ return (int32)(fNumber - sequence) > 0; }
+		fNumber = sequence.fNumber;
-			bool	operator>=(uint32 sequence) const
+		return *this;
-						{ return (int32)(fNumber - sequence) >= 0; }
+	}
-			bool	operator<(uint32 sequence) const
-						{ return (int32)(fNumber - sequence) < 0; }
-			bool	operator<=(uint32 sequence) const
-						{ return (int32)(fNumber - sequence) <= 0; }
 
-			uint32&	operator+=(uint32 sequence) { return fNumber += sequence; }
+	inline tcp_sequence& operator+=(tcp_sequence sequence)
-			uint32&	operator++() { return ++fNumber; }
+	{
-			uint32	operator++(int _) { return fNumber++; }
+		fNumber += sequence.fNumber;
+		return *this;
+	}
 
-private:
+	inline tcp_sequence& operator++()
+	{
+		fNumber++;
+		return *this;
+	}
+
+	inline tcp_sequence operator++(int _)
+	{
+		fNumber++;
+		return fNumber - 1;
+	}
+
+// Conceptually private, but used in global operators.
+//private:
 	uint32	fNumber;
 };
 
+
+// Global tcp_sequence Operators
+
+
+inline bool
+operator>(tcp_sequence a, tcp_sequence b)
+{
+	return (int32)(a.fNumber - b.fNumber) > 0;
+}
+
+
+inline bool
+operator>=(tcp_sequence a, tcp_sequence b)
+{
+	return (int32)(a.fNumber - b.fNumber) >= 0;
+}
+
+
+inline bool
+operator<(tcp_sequence a, tcp_sequence b)
+{
+	return (int32)(a.fNumber - b.fNumber) < 0;
+}
+
+
+inline bool
+operator<=(tcp_sequence a, tcp_sequence b)
+{
+	return (int32)(a.fNumber - b.fNumber) <= 0;
+}
+
+
+inline tcp_sequence
+operator+(tcp_sequence a, tcp_sequence b)
+{
+	return a.fNumber + b.fNumber;
+}
+
+
+inline tcp_sequence
+operator-(tcp_sequence a, tcp_sequence b)
+{
+	return a.fNumber - b.fNumber;
+}
+
+
+inline bool
+operator!=(tcp_sequence a, tcp_sequence b)
+{
+	return a.fNumber != b.fNumber;
+}
+
+
+inline bool
+operator==(tcp_sequence a, tcp_sequence b)
+{
+	return a.fNumber == b.fNumber;
+}
+
+
 // TCP flag constants
 #define TCP_FLAG_FINISH					0x01
 #define TCP_FLAG_SYNCHRONIZE			0x02