[Mono-dev] [PATCH] Add GetString to UnicodeEncoding 2.0 andmodifysome Encoding wrappers

[Atsushi Eno]

Hello,

> This is the revised version of my original patch. Please approve this 
> one and the other things can be modified (if needed) later.

Comments inline (btw as it is often asked, text/plain is better than
application/octet-stream for patch attachments for review).

> @@ -146,8 +146,7 @@
>  				return GetBytesInternal (charPtr + charIndex, charCount, bytePtr + byteIndex, byteCount);
>  	}
>  
> -#if !NET_2_0
> -	public unsafe override byte [] GetBytes (String s)
> +	public override byte [] GetBytes (String s)
>  	{
>  		if (s == null)
>  			throw new ArgumentNullException ("s");
> @@ -155,14 +154,10 @@
>  		int byteCount = GetByteCount (s);
>  		byte [] bytes = new byte [byteCount];
>  
> -		if (byteCount != 0)
> -			fixed (char* charPtr = s)
> -				fixed (byte* bytePtr = bytes)
> -					GetBytesInternal (charPtr, s.Length, bytePtr, byteCount);
> +		GetBytes (s, 0, s.Length, bytes, 0);
>  
>  		return bytes;
>  	}
> -#endif
>  
>  	public unsafe override int GetBytes (String s, int charIndex, int charCount,
>  										byte [] bytes, int byteIndex)

Why do you *dare* make API mismatch here? Don't remove #if NET_2_0.
It does not improve anything.

> @@ -300,6 +299,30 @@
>  	}
>  #endif
>  
> +	// Decode a buffer of bytes into a string.
> +	public unsafe override String GetString (byte [] bytes, int index, int count)
> +	{
> +		if (bytes == null)
> +			throw new ArgumentNullException ("bytes");
> +		if (index < 0 || index > bytes.Length)
> +			throw new ArgumentOutOfRangeException ("index", _("ArgRange_Array"));
> +		if (count < 0 || count > (bytes.Length - index))
> +			throw new ArgumentOutOfRangeException ("count", _("ArgRange_Array"));
> +
> +		if (count == 0)
> +			return string.Empty;
> +
> +		// GetCharCountInternal
> +		int charCount = count / 2;
> +		string s = string.InternalAllocateStr (charCount);
> +
> +		fixed (byte* bytePtr = bytes)
> +			fixed (char* charPtr = s)
> +				GetCharsInternal (bytePtr + index, count, charPtr, charCount);
> +
> +		return s;
> +	}
> +
>  	private unsafe int GetCharsInternal (byte* bytes, int byteCount,
>  										char* chars, int charCount)
>  	{

So, this is the part that you said it is (will be) overriden only in
Mono (for 1.1), right? I think this 1/2 memory consumption is awesome :)

Other than the first point it looks good. Thanks Kornél.

Atsushi Eno