Skip to content

Commit fe958fa

Browse files
committed
address feedback
1 parent af5ac30 commit fe958fa

File tree

7 files changed

+29
-40
lines changed

7 files changed

+29
-40
lines changed

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4141,7 +4141,7 @@ private TdsOperationStatus TryResetBlobState()
41414141
#if DEBUG
41424142
else
41434143
{
4144-
//Debug.Assert((_sharedState._columnDataBytesRemaining == 0 || _sharedState._columnDataBytesRemaining == -1) && _stateObj._longlen == 0, "Haven't read header yet, but column is partially read?");
4144+
Debug.Assert((_sharedState._columnDataBytesRemaining == 0 || _sharedState._columnDataBytesRemaining == -1) && _stateObj._longlen == 0, "Haven't read header yet, but column is partially read?");
41454145
}
41464146
#endif
41474147

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3565,7 +3565,6 @@ private TdsOperationStatus TryProcessDataClassification(TdsParserStateObject sta
35653565
int sensitivityRank = (int)SensitivityRank.NOT_DEFINED;
35663566
if (DataClassificationVersion > TdsEnums.DATA_CLASSIFICATION_VERSION_WITHOUT_RANK_SUPPORT)
35673567
{
3568-
35693568
result = stateObj.TryReadInt32(out sensitivityRank);
35703569
if (result != TdsOperationStatus.Done)
35713570
{
@@ -3631,7 +3630,6 @@ private TdsOperationStatus TryProcessDataClassification(TdsParserStateObject sta
36313630
int sensitivityRankProperty = (int)SensitivityRank.NOT_DEFINED;
36323631
if (DataClassificationVersion > TdsEnums.DATA_CLASSIFICATION_VERSION_WITHOUT_RANK_SUPPORT)
36333632
{
3634-
36353633
result = stateObj.TryReadInt32(out sensitivityRankProperty);
36363634
if (result != TdsOperationStatus.Done)
36373635
{

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -869,7 +869,7 @@ private TdsOperationStatus TryCleanPartialRead()
869869
{
870870
byte token;
871871
TdsOperationStatus debugResult = _stateObj.TryPeekByte(out token);
872-
if ( debugResult != TdsOperationStatus.Done)
872+
if (debugResult != TdsOperationStatus.Done)
873873
{
874874
return debugResult;
875875
}
@@ -1094,7 +1094,7 @@ private TdsOperationStatus TryCloseInternal(bool closeReader)
10941094
{
10951095
cleanDataFailed = true;
10961096
result = TryCleanPartialRead();
1097-
if (result != TdsOperationStatus.Done)
1097+
if (result == TdsOperationStatus.Done)
10981098
{
10991099
cleanDataFailed = false;
11001100
}
@@ -4685,7 +4685,7 @@ private TdsOperationStatus TryResetBlobState()
46854685
#if DEBUG
46864686
else
46874687
{
4688-
//Debug.Assert((_sharedState._columnDataBytesRemaining == 0 || _sharedState._columnDataBytesRemaining == -1) && _stateObj._longlen == 0, "Haven't read header yet, but column is partially read?");
4688+
Debug.Assert((_sharedState._columnDataBytesRemaining == 0 || _sharedState._columnDataBytesRemaining == -1) && _stateObj._longlen == 0, "Haven't read header yet, but column is partially read?");
46894689
}
46904690
#endif
46914691

@@ -5540,7 +5540,7 @@ private static Task<bool> ReadAsyncExecute(Task task, object state)
55405540

55415541
// if non-sequentialaccess then read entire row before returning
55425542
result = reader.TryReadColumn(reader._metaData.Length - 1, true);
5543-
if (result != TdsOperationStatus.Done)
5543+
if (result == TdsOperationStatus.Done)
55445544
{
55455545
// completed
55465546
return ADP.TrueTask;

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Packet.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
using System;
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
26

37
namespace Microsoft.Data.SqlClient
48
{

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
using System;
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
26
using System.Diagnostics;
37

48
namespace Microsoft.Data.SqlClient
@@ -33,9 +37,6 @@ public void ProcessSniPacket(PacketHandle packet, uint error, bool usePartialPac
3337

3438
if (usePartialPacket && _snapshot == null && _partialPacket != null && _partialPacket.IsComplete)
3539
{
36-
//Debug.Assert(_snapshot == null, "_snapshot must be null when processing partial packet instead of network read");
37-
//Debug.Assert(_partialPacket != null, "_partialPacket must not be null when usePartialPacket is true");
38-
//Debug.Assert(_partialPacket.IsComplete, "_partialPacket.IsComplete must be true to use it in place of a real read");
3940
SetBuffer(_partialPacket.Buffer, 0, _partialPacket.CurrentLength);
4041
ClearPartialPacket();
4142
getDataError = TdsEnums.SNI_SUCCESS;
@@ -147,12 +148,6 @@ out recurse
147148
if (_snapshotStatus != SnapshotStatus.NotActive && appended)
148149
{
149150
_snapshot.MoveNext();
150-
#if DEBUG
151-
// multiple packets can be appended by demuxing but we should only move
152-
// forward by a single packet so we can no longer assert that we are on
153-
// the last packet at this time
154-
//_snapshot.AssertCurrent();
155-
#endif
156151
}
157152
}
158153

@@ -216,10 +211,10 @@ out bool recurse
216211
if (!partialPacket.HasDataLength)
217212
{
218213
// we need to get enough bytes to read the packet header
219-
int headeBytesNeeded = Math.Max(0, TdsEnums.HEADER_LEN - partialPacket.CurrentLength);
220-
if (headeBytesNeeded > 0)
214+
int headerBytesNeeded = Math.Max(0, TdsEnums.HEADER_LEN - partialPacket.CurrentLength);
215+
if (headerBytesNeeded > 0)
221216
{
222-
int headerBytesAvailable = Math.Min(data.Length, headeBytesNeeded);
217+
int headerBytesAvailable = Math.Min(data.Length, headerBytesNeeded);
223218

224219
Span<byte> headerTarget = partialPacket.Buffer.AsSpan(partialPacket.CurrentLength, headerBytesAvailable);
225220
ReadOnlySpan<byte> headerSource = data.Slice(0, headerBytesAvailable);
@@ -255,7 +250,7 @@ out bool recurse
255250
}
256251
else if (partialPacket.CurrentLength > partialPacket.RequiredLength)
257252
{
258-
// the partial packet contains a complete packet of data and then and also contains
253+
// the partial packet contains a complete packet of data and also contains
259254
// data from a following packet
260255

261256
// the TDS spec requires that all packets be of the defined packet size apart from
@@ -334,8 +329,8 @@ out bool recurse
334329
CurrentLength = data.Length
335330
};
336331
consumeRemainderPacket = true;
337-
//Debug.Assert(remainderPacket.HasHeader); // precondition of entering this block
338-
//Debug.Assert(remainderPacket.HasDataLength); // must have been set at construction
332+
Debug.Assert(remainderPacket.HasHeader); // precondition of entering this block
333+
Debug.Assert(remainderPacket.HasDataLength); // must have been set at construction
339334
if (remainderPacket.CurrentLength >= remainderPacket.RequiredLength)
340335
{
341336
// the remainder packet contains more data than the packet so we need
@@ -359,9 +354,9 @@ out bool recurse
359354
}
360355
else // implied: current length > required length
361356
{
362-
//// more data than required so need to split it out but we can't do that
363-
//// here so we need to tell the caller to take the remainer packet and then
364-
//// call this function again
357+
// more data than required so need to split it out but we can't do that
358+
// here so we need to tell the caller to take the remainder packet and then
359+
// call this function again
365360

366361
int remainderLength = data.Length - (TdsEnums.HEADER_LEN + packetDataLength);
367362
remainderPacket = new Packet

src/Microsoft.Data.SqlClient/tests/FunctionalTests/MultiplexerTests.cs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
using System.Collections.Generic;
44
using System.Diagnostics;
55
using System.Diagnostics.CodeAnalysis;
6-
using System.Linq;
76
using System.Text;
8-
using System.Threading.Tasks;
97
using Xunit;
108

119
namespace Microsoft.Data.SqlClient.Tests
@@ -176,7 +174,6 @@ public static void FailReconstruct2Packets_FullFullPart_Part(bool isAsync)
176174
Assert.Throws<Exception>(
177175
() => MultiplexPacketList(isAsync, maxDataSize, input)
178176
);
179-
180177
}
181178

182179

@@ -208,7 +205,6 @@ private static List<PacketData> MultiplexPacketList(bool isAsync, int dataSize,
208205
}
209206
}
210207

211-
212208
if (!isAsync)
213209
{
214210
if (stateObject._partialPacket != null)
@@ -337,7 +333,6 @@ public static List<PacketData> SplitPackets(int dataSize, List<PacketData> packe
337333
int sourceOffset = 0;
338334
int sourceIndex = 0;
339335

340-
341336
do
342337
{
343338
Span<byte> targetSpan = Span<byte>.Empty;
@@ -371,8 +366,6 @@ public static List<PacketData> SplitPackets(int dataSize, List<PacketData> packe
371366
sourceOffset += copy;
372367
sourceSpan.Slice(0, copy).CopyTo(targetSpan.Slice(0, copy));
373368
}
374-
375-
376369
} while (sourceIndex < packets.Count && targetIndex < arrays.Length);
377370

378371
foreach (var array in arrays)
@@ -383,7 +376,6 @@ public static List<PacketData> SplitPackets(int dataSize, List<PacketData> packe
383376
return list;
384377
}
385378

386-
387379
public static int PacketSizeFromDataSize(int dataSize) => TdsEnums.HEADER_LEN + dataSize;
388380

389381
public static int DataSizeFromPacketSize(int packetSize) => packetSize - TdsEnums.HEADER_LEN;
@@ -465,7 +457,6 @@ public string ToDebugString()
465457
}
466458
return buffer.ToString();
467459
}
468-
469460
}
470461

471462
[DebuggerStepThrough]

src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
using System;
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
26
using System.Collections.Generic;
37
using System.Diagnostics;
48
using Microsoft.Data.SqlClient.Tests;
@@ -128,14 +132,11 @@ void SetBuffer(byte[] buffer, int inBytesUsed, int inBytesRead)
128132
_inBytesRead = inBytesRead;
129133
}
130134

131-
132-
133135
// stubs
134136
private LastIOTimer _lastSuccessfulIOTimer = new LastIOTimer();
135137
private Parser _parser = new Parser();
136138
private SnapshotStatus _snapshotStatus = SnapshotStatus.NotActive;
137139

138-
139140
[DebuggerStepThrough]
140141
private void SniReadStatisticsAndTracing() { }
141142
[DebuggerStepThrough]

0 commit comments

Comments
 (0)