linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] lib/ucs2_string: Correct ucs2 -> utf8 conversion
@ 2016-02-12 23:13 Jason Andryuk
  2016-02-15 11:12 ` Matt Fleming
  0 siblings, 1 reply; 3+ messages in thread
From: Jason Andryuk @ 2016-02-12 23:13 UTC (permalink / raw)
  To: Peter Jones, Matt Fleming, linux-kernel; +Cc: Matthew Garrett, Jason Andryuk

The comparisons should be >= since 0x800 and 0x80 require an additional bit
to store.

For the 3 byte case, the existing shift would drop off 2 more bits than
intended.

For the 2 byte case, there should be 5 bits bits in byte 1, and 6 bits in
byte 2.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---

Tested in user space, but not in the kernel.  Conversions now match
python's unicode conversions.

 lib/ucs2_string.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/ucs2_string.c b/lib/ucs2_string.c
index 17dd74e..f0b323a 100644
--- a/lib/ucs2_string.c
+++ b/lib/ucs2_string.c
@@ -59,9 +59,9 @@ ucs2_utf8size(const ucs2_char_t *src)
 	for (i = 0; i < ucs2_strlen(src); i++) {
 		u16 c = src[i];
 
-		if (c > 0x800)
+		if (c >= 0x800)
 			j += 3;
-		else if (c > 0x80)
+		else if (c >= 0x80)
 			j += 2;
 		else
 			j += 1;
@@ -88,19 +88,19 @@ ucs2_as_utf8(u8 *dest, const ucs2_char_t *src, unsigned long maxlength)
 	for (i = 0; maxlength && i < limit; i++) {
 		u16 c = src[i];
 
-		if (c > 0x800) {
+		if (c >= 0x800) {
 			if (maxlength < 3)
 				break;
 			maxlength -= 3;
 			dest[j++] = 0xe0 | (c & 0xf000) >> 12;
-			dest[j++] = 0x80 | (c & 0x0fc0) >> 8;
+			dest[j++] = 0x80 | (c & 0x0fc0) >> 6;
 			dest[j++] = 0x80 | (c & 0x003f);
-		} else if (c > 0x80) {
+		} else if (c >= 0x80) {
 			if (maxlength < 2)
 				break;
 			maxlength -= 2;
-			dest[j++] = 0xc0 | (c & 0xfe0) >> 5;
-			dest[j++] = 0x80 | (c & 0x01f);
+			dest[j++] = 0xc0 | (c & 0x7c0) >> 6;
+			dest[j++] = 0x80 | (c & 0x03f);
 		} else {
 			maxlength -= 1;
 			dest[j++] = c & 0x7f;
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] lib/ucs2_string: Correct ucs2 -> utf8 conversion
  2016-02-12 23:13 [PATCH] lib/ucs2_string: Correct ucs2 -> utf8 conversion Jason Andryuk
@ 2016-02-15 11:12 ` Matt Fleming
  2016-02-15 12:19   ` Laszlo Ersek
  0 siblings, 1 reply; 3+ messages in thread
From: Matt Fleming @ 2016-02-15 11:12 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: Peter Jones, linux-kernel, Matthew Garrett, Laszlo Ersek,
	linux-efi

(Cc'ing Laszlo and linux-efi)

On Fri, 12 Feb, at 11:13:33PM, Jason Andryuk wrote:
> The comparisons should be >= since 0x800 and 0x80 require an additional bit
> to store.
> 
> For the 3 byte case, the existing shift would drop off 2 more bits than
> intended.
> 
> For the 2 byte case, there should be 5 bits bits in byte 1, and 6 bits in
> byte 2.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
> 
> Tested in user space, but not in the kernel.  Conversions now match
> python's unicode conversions.
> 
>  lib/ucs2_string.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
 
Thanks Jason. Peter, Laszlo, any comments?

> diff --git a/lib/ucs2_string.c b/lib/ucs2_string.c
> index 17dd74e..f0b323a 100644
> --- a/lib/ucs2_string.c
> +++ b/lib/ucs2_string.c
> @@ -59,9 +59,9 @@ ucs2_utf8size(const ucs2_char_t *src)
>  	for (i = 0; i < ucs2_strlen(src); i++) {
>  		u16 c = src[i];
>  
> -		if (c > 0x800)
> +		if (c >= 0x800)
>  			j += 3;
> -		else if (c > 0x80)
> +		else if (c >= 0x80)
>  			j += 2;
>  		else
>  			j += 1;
> @@ -88,19 +88,19 @@ ucs2_as_utf8(u8 *dest, const ucs2_char_t *src, unsigned long maxlength)
>  	for (i = 0; maxlength && i < limit; i++) {
>  		u16 c = src[i];
>  
> -		if (c > 0x800) {
> +		if (c >= 0x800) {
>  			if (maxlength < 3)
>  				break;
>  			maxlength -= 3;
>  			dest[j++] = 0xe0 | (c & 0xf000) >> 12;
> -			dest[j++] = 0x80 | (c & 0x0fc0) >> 8;
> +			dest[j++] = 0x80 | (c & 0x0fc0) >> 6;
>  			dest[j++] = 0x80 | (c & 0x003f);
> -		} else if (c > 0x80) {
> +		} else if (c >= 0x80) {
>  			if (maxlength < 2)
>  				break;
>  			maxlength -= 2;
> -			dest[j++] = 0xc0 | (c & 0xfe0) >> 5;
> -			dest[j++] = 0x80 | (c & 0x01f);
> +			dest[j++] = 0xc0 | (c & 0x7c0) >> 6;
> +			dest[j++] = 0x80 | (c & 0x03f);
>  		} else {
>  			maxlength -= 1;
>  			dest[j++] = c & 0x7f;
> -- 
> 2.4.3
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] lib/ucs2_string: Correct ucs2 -> utf8 conversion
  2016-02-15 11:12 ` Matt Fleming
@ 2016-02-15 12:19   ` Laszlo Ersek
  0 siblings, 0 replies; 3+ messages in thread
From: Laszlo Ersek @ 2016-02-15 12:19 UTC (permalink / raw)
  To: Matt Fleming, Jason Andryuk
  Cc: Peter Jones, linux-kernel, Matthew Garrett, linux-efi

On 02/15/16 12:12, Matt Fleming wrote:
> (Cc'ing Laszlo and linux-efi)
> 
> On Fri, 12 Feb, at 11:13:33PM, Jason Andryuk wrote:
>> The comparisons should be >= since 0x800 and 0x80 require an additional bit
>> to store.
>>
>> For the 3 byte case, the existing shift would drop off 2 more bits than
>> intended.
>>
>> For the 2 byte case, there should be 5 bits bits in byte 1, and 6 bits in
>> byte 2.
>>
>> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
>> ---
>>
>> Tested in user space, but not in the kernel.  Conversions now match
>> python's unicode conversions.
>>
>>  lib/ucs2_string.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>  
> Thanks Jason. Peter, Laszlo, any comments?
> 
>> diff --git a/lib/ucs2_string.c b/lib/ucs2_string.c
>> index 17dd74e..f0b323a 100644
>> --- a/lib/ucs2_string.c
>> +++ b/lib/ucs2_string.c
>> @@ -59,9 +59,9 @@ ucs2_utf8size(const ucs2_char_t *src)
>>  	for (i = 0; i < ucs2_strlen(src); i++) {
>>  		u16 c = src[i];
>>  
>> -		if (c > 0x800)
>> +		if (c >= 0x800)
>>  			j += 3;
>> -		else if (c > 0x80)
>> +		else if (c >= 0x80)
>>  			j += 2;
>>  		else
>>  			j += 1;

This change looks justified, from the table at

  https://en.wikipedia.org/wiki/UTF-8#Description

>> @@ -88,19 +88,19 @@ ucs2_as_utf8(u8 *dest, const ucs2_char_t *src, unsigned long maxlength)
>>  	for (i = 0; maxlength && i < limit; i++) {
>>  		u16 c = src[i];
>>  
>> -		if (c > 0x800) {
>> +		if (c >= 0x800) {
>>  			if (maxlength < 3)
>>  				break;
>>  			maxlength -= 3;
>>  			dest[j++] = 0xe0 | (c & 0xf000) >> 12;

Okay, so byte #1 consumes the most significant 4 bits

>> -			dest[j++] = 0x80 | (c & 0x0fc0) >> 8;
>> +			dest[j++] = 0x80 | (c & 0x0fc0) >> 6;

Byte #2 is supposed to consume 6 more bits:

    1234 56
00001111 11000000  binary
0   f    c   0     hex -> mask is okay

Indeed the shift count should be 6.

>>  			dest[j++] = 0x80 | (c & 0x003f);
>> -		} else if (c > 0x80) {
>> +		} else if (c >= 0x80) {
>>  			if (maxlength < 2)
>>  				break;
>>  			maxlength -= 2;
>> -			dest[j++] = 0xc0 | (c & 0xfe0) >> 5;
>> -			dest[j++] = 0x80 | (c & 0x01f);
>> +			dest[j++] = 0xc0 | (c & 0x7c0) >> 6;

Byte #1 is supposed to consume the 5 most significant bits, from the 11
bits that the code point has:

00000111 11111111 -- bin
0   7    f   f    -- hex -- all it can have

     123 45
00000111 11000000 -- bin
0   7    c   0    -- hex -- mask is okay

Shift count of 6 looks okay.


>> +			dest[j++] = 0x80 | (c & 0x03f);

Byte #2 is supposed to consume the remaining 6 bits:

           123456
00000000 00111111 -- bin
0   0    3   f    -- hex - mask is okay

Maybe if we could write the mask as 0x3f, instead of 0x03f.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

>>  		} else {
>>  			maxlength -= 1;
>>  			dest[j++] = c & 0x7f;
>> -- 
>> 2.4.3
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-02-15 12:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-12 23:13 [PATCH] lib/ucs2_string: Correct ucs2 -> utf8 conversion Jason Andryuk
2016-02-15 11:12 ` Matt Fleming
2016-02-15 12:19   ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).