Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Zero length check #2765

Closed
wants to merge 1 commit into from
Closed

Fix Zero length check #2765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/StackExchange.Redis/PhysicalConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using Pipelines.Sockets.Unofficial;
using Microsoft.Extensions.Logging;
using Pipelines.Sockets.Unofficial;
using Pipelines.Sockets.Unofficial.Arenas;
using System;
using System.Buffers;
using System.Buffers.Binary;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
Expand All @@ -17,9 +19,7 @@
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using static StackExchange.Redis.Message;
using System.Buffers.Binary;

namespace StackExchange.Redis
{
Expand Down Expand Up @@ -91,7 +91,7 @@ public PhysicalConnection(PhysicalBridge bridge)
connectionType = bridge.ConnectionType;
_bridge = new WeakReference(bridge);
ChannelPrefix = bridge.Multiplexer.RawConfig.ChannelPrefix;
if (ChannelPrefix?.Length == 0) ChannelPrefix = null; // null tests are easier than null+empty
if (ChannelPrefix == null || ChannelPrefix.Length == 0) ChannelPrefix = null; // null tests are easier than null+empty
var endpoint = bridge.ServerEndPoint.EndPoint;
_physicalName = connectionType + "#" + Interlocked.Increment(ref totalCount) + "@" + Format.ToString(endpoint);

Expand Down
6 changes: 3 additions & 3 deletions src/StackExchange.Redis/RedisKey.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace StackExchange.Redis
{
internal RedisKey(byte[]? keyPrefix, object? keyValue)
{
KeyPrefix = keyPrefix?.Length == 0 ? null : keyPrefix;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's compare using:

using System;

Original(null);
Original("");
Original("abc");

Proposed(null);
Proposed("");
Proposed("abc");

static void Original(string? keyPrefix) => Write(keyPrefix?.Length == 0 ? null : keyPrefix);
static void Proposed(string? keyPrefix) => Write(keyPrefix == null || keyPrefix.Length == 0 ? null : keyPrefix);

static void Write(string? value)
    => Console.WriteLine(value is null ? "(null)" : "\"" + value + "\"");

outputs:

keyPrefix input original output proposed output
null null null
"" null null
"abc" "abc" "abc"

So... this has no functional change

KeyPrefix = keyPrefix == null || keyPrefix.Length == 0 ? null : keyPrefix;
KeyValue = keyValue;
}

Expand Down Expand Up @@ -253,7 +253,7 @@ public static implicit operator RedisKey(byte[]? key)
/// Obtain the <see cref="RedisKey"/> as a <see cref="T:byte[]"/>.
/// </summary>
/// <param name="key">The key to get a byte array for.</param>
public static implicit operator byte[]? (RedisKey key)
public static implicit operator byte[]?(RedisKey key)
{
if (key.IsNull) return null;
if (key.TryGetSimpleBuffer(out var arr)) return arr;
Expand All @@ -270,7 +270,7 @@ public static implicit operator RedisKey(byte[]? key)
/// Obtain the key as a <see cref="string"/>.
/// </summary>
/// <param name="key">The key to get a string for.</param>
public static implicit operator string? (RedisKey key)
public static implicit operator string?(RedisKey key)
{
if (key.KeyPrefix is null)
{
Expand Down
2 changes: 1 addition & 1 deletion src/StackExchange.Redis/RedisResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ internal override bool AsBoolean()
: Array.ConvertAll(_value, x => x.AsByteArray()!);

private bool IsSingleton => _value?.Length == 1;
private bool IsEmpty => _value?.Length == 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this changes the value of IsEmpty for null values of _value; currently a null will return false for IsEmpty; with this change it will return true. The question is what should IsEmpty return for null values? IMO we should retain the current behaviour, where null reports false here (note the separate IsNull test). Changing the result of this property is a significant proposition that would require justification.

private bool IsEmpty => _value == null || _value.Length == 0;
internal override double AsDouble()
{
if (IsSingleton) return _value![0].AsDouble();
Expand Down
Loading