diff mbox series

[BlueZ,v3] gobex: Replace g_convert by utf16_to_utf8

Message ID 20240910090201.115557-1-frederic.danis@collabora.com (mailing list archive)
State Superseded
Headers show
Series [BlueZ,v3] gobex: Replace g_convert by utf16_to_utf8 | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck fail BlueZ Make Check FAIL:
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild warning ScanBuild: gobex/gobex-header.c:95:2: warning: Null pointer passed to 2nd parameter expecting 'nonnull' memcpy(to, from, count); ^~~~~~~~~~~~~~~~~~~~~~~ 1 warning generated.

Commit Message

Frédéric Danis Sept. 10, 2024, 9:02 a.m. UTC
The glibc's iconv implementation is based around plug in modules
for specific translations which may not been built on the platform
and prevent to use g_convert().
This commit replaces it by a function similar to the existing
utf8_to_utf16() function.
---
v1 -> v2: Fix missing g_free
v2 -> v3: Replace g_malloc0 by alloca
          Fix UTF-16 buffer size allocation
          Ensure that UTF-16 buffer is terminated by '\0'

 gobex/gobex-header.c | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

Comments

bluez.test.bot@gmail.com Sept. 10, 2024, 10:46 a.m. UTC | #1
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=888782

---Test result---

Test Summary:
CheckPatch                    PASS      0.46 seconds
GitLint                       PASS      0.29 seconds
BuildEll                      PASS      25.24 seconds
BluezMake                     PASS      1670.15 seconds
MakeCheck                     FAIL      10.99 seconds
MakeDistcheck                 PASS      177.86 seconds
CheckValgrind                 PASS      253.85 seconds
CheckSmatch                   PASS      359.61 seconds
bluezmakeextell               PASS      120.52 seconds
IncrementalBuild              PASS      1623.29 seconds
ScanBuild                     WARNING   1051.26 seconds

Details
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:

make[3]: *** [Makefile:11773: test-suite.log] Error 1
make[2]: *** [Makefile:11881: check-TESTS] Error 2
make[1]: *** [Makefile:12310: check-am] Error 2
make: *** [Makefile:12312: check] Error 2
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
gobex/gobex-header.c:95:2: warning: Null pointer passed to 2nd parameter expecting 'nonnull'
        memcpy(to, from, count);
        ^~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Sept. 10, 2024, 2:48 p.m. UTC | #2
Hi Frédéric,

On Tue, Sep 10, 2024 at 5:03 AM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> The glibc's iconv implementation is based around plug in modules
> for specific translations which may not been built on the platform
> and prevent to use g_convert().
> This commit replaces it by a function similar to the existing
> utf8_to_utf16() function.
> ---
> v1 -> v2: Fix missing g_free
> v2 -> v3: Replace g_malloc0 by alloca
>           Fix UTF-16 buffer size allocation
>           Ensure that UTF-16 buffer is terminated by '\0'
>
>  gobex/gobex-header.c | 41 +++++++++++++++++++++++++++++++++++------
>  1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/gobex/gobex-header.c b/gobex/gobex-header.c
> index 002ba8861..889c794df 100644
> --- a/gobex/gobex-header.c
> +++ b/gobex/gobex-header.c
> @@ -62,6 +62,34 @@ static glong utf8_to_utf16(gunichar2 **utf16, const char *utf8) {
>         return utf16_len;
>  }
>
> +static glong utf16_to_utf8(char **utf8, const gunichar2 *utf16, guint16 len,
> +                               GError **err)
> +{
> +       glong utf8_len;
> +       guint16 utf16_len, i;
> +       gunichar2 *buf;
> +
> +       if (*utf16 == '\0') {
> +               *utf8 = NULL;
> +               return 0;
> +       }
> +
> +       /* OBEX requires network byteorder (big endian) UTF-16
> +        * but g_utf16_to_utf8 expects host-byteorder UTF-8
> +        */
> +       utf16_len = len / sizeof(gunichar2);
> +       buf = alloca(sizeof(gunichar2) * utf16_len);
> +       for (i = 0; i < utf16_len; i++)
> +               (buf)[i] = g_ntohs(utf16[i]);
> +       buf[utf16_len] = '\0';

You will need to allocate one extra index if you want it to be NULL
terminated otherwise you will be accessing past allocated area, that
said can't you just pass the utf16_len to g_utf16_to_utf8 instead of
-1?

> +
> +       *utf8 = g_utf16_to_utf8(buf, -1, NULL, &utf8_len, err);
> +       if (*utf8 == NULL)
> +               utf8_len = -1;
> +
> +       return utf8_len;
> +}
> +
>  static guint8 *put_bytes(guint8 *to, const void *from, gsize count)
>  {
>         memcpy(to, from, count);
> @@ -130,7 +158,7 @@ GObexHeader *g_obex_header_decode(const void *data, gsize len,
>         GObexHeader *header;
>         const guint8 *ptr = data;
>         guint16 hdr_len;
> -       gsize str_len;
> +       glong str_len;
>         GError *conv_err = NULL;
>
>         if (len < 2) {
> @@ -177,13 +205,14 @@ GObexHeader *g_obex_header_decode(const void *data, gsize len,
>                         goto failed;
>                 }
>
> -               header->v.string = g_convert((const char *) ptr, hdr_len - 5,
> -                                               "UTF-8", "UTF-16BE",
> -                                               NULL, &str_len, &conv_err);
> -               if (header->v.string == NULL) {
> +               str_len = utf16_to_utf8(&header->v.string,
> +                                       (const gunichar2 *) ptr,
> +                                       hdr_len - 5,
> +                                       &conv_err);
> +               if (str_len < 0) {
>                         g_set_error(err, G_OBEX_ERROR,
>                                         G_OBEX_ERROR_PARSE_ERROR,
> -                                       "Unicode conversion failed: %s",
> +                                       "UTF16 to UTF8 conversion failed: %s",
>                                         conv_err->message);
>                         g_error_free(conv_err);
>                         goto failed;
> --
> 2.34.1
>
>
Frédéric Danis Sept. 10, 2024, 3 p.m. UTC | #3
Hi Luiz,

On 10/09/2024 16:48, Luiz Augusto von Dentz wrote:
> Hi Frédéric,
>
> On Tue, Sep 10, 2024 at 5:03 AM Frédéric Danis
> <frederic.danis@collabora.com> wrote:
>> The glibc's iconv implementation is based around plug in modules
>> for specific translations which may not been built on the platform
>> and prevent to use g_convert().
>> This commit replaces it by a function similar to the existing
>> utf8_to_utf16() function.
>> ---
>> v1 -> v2: Fix missing g_free
>> v2 -> v3: Replace g_malloc0 by alloca
>>            Fix UTF-16 buffer size allocation
>>            Ensure that UTF-16 buffer is terminated by '\0'
>>
>>   gobex/gobex-header.c | 41 +++++++++++++++++++++++++++++++++++------
>>   1 file changed, 35 insertions(+), 6 deletions(-)
>>
>> diff --git a/gobex/gobex-header.c b/gobex/gobex-header.c
>> index 002ba8861..889c794df 100644
>> --- a/gobex/gobex-header.c
>> +++ b/gobex/gobex-header.c
>> @@ -62,6 +62,34 @@ static glong utf8_to_utf16(gunichar2 **utf16, const char *utf8) {
>>          return utf16_len;
>>   }
>>
>> +static glong utf16_to_utf8(char **utf8, const gunichar2 *utf16, guint16 len,
>> +                               GError **err)
>> +{
>> +       glong utf8_len;
>> +       guint16 utf16_len, i;
>> +       gunichar2 *buf;
>> +
>> +       if (*utf16 == '\0') {
>> +               *utf8 = NULL;
>> +               return 0;
>> +       }
>> +
>> +       /* OBEX requires network byteorder (big endian) UTF-16
>> +        * but g_utf16_to_utf8 expects host-byteorder UTF-8
>> +        */
>> +       utf16_len = len / sizeof(gunichar2);
>> +       buf = alloca(sizeof(gunichar2) * utf16_len);
>> +       for (i = 0; i < utf16_len; i++)
>> +               (buf)[i] = g_ntohs(utf16[i]);
>> +       buf[utf16_len] = '\0';
> You will need to allocate one extra index if you want it to be NULL
> terminated otherwise you will be accessing past allocated area, that
> said can't you just pass the utf16_len to g_utf16_to_utf8 instead of
> -1?

iiuc g_utf16_to_utf8() doc, if the source len is passed the dest string 
may include embedded NUL characters or none, while when passing -1 it 
will stop at the first NUL character.I will update allocation length.
>> +
>> +       *utf8 = g_utf16_to_utf8(buf, -1, NULL, &utf8_len, err);
>> +       if (*utf8 == NULL)
>> +               utf8_len = -1;
>> +
>> +       return utf8_len;
>> +}
>> +
>>   static guint8 *put_bytes(guint8 *to, const void *from, gsize count)
>>   {
>>          memcpy(to, from, count);
>> @@ -130,7 +158,7 @@ GObexHeader *g_obex_header_decode(const void *data, gsize len,
>>          GObexHeader *header;
>>          const guint8 *ptr = data;
>>          guint16 hdr_len;
>> -       gsize str_len;
>> +       glong str_len;
>>          GError *conv_err = NULL;
>>
>>          if (len < 2) {
>> @@ -177,13 +205,14 @@ GObexHeader *g_obex_header_decode(const void *data, gsize len,
>>                          goto failed;
>>                  }
>>
>> -               header->v.string = g_convert((const char *) ptr, hdr_len - 5,
>> -                                               "UTF-8", "UTF-16BE",
>> -                                               NULL, &str_len, &conv_err);
>> -               if (header->v.string == NULL) {
>> +               str_len = utf16_to_utf8(&header->v.string,
>> +                                       (const gunichar2 *) ptr,
>> +                                       hdr_len - 5,
>> +                                       &conv_err);
>> +               if (str_len < 0) {
>>                          g_set_error(err, G_OBEX_ERROR,
>>                                          G_OBEX_ERROR_PARSE_ERROR,
>> -                                       "Unicode conversion failed: %s",
>> +                                       "UTF16 to UTF8 conversion failed: %s",
>>                                          conv_err->message);
>>                          g_error_free(conv_err);
>>                          goto failed;
>> --
>> 2.34.1
>>
>>
>
diff mbox series

Patch

diff --git a/gobex/gobex-header.c b/gobex/gobex-header.c
index 002ba8861..889c794df 100644
--- a/gobex/gobex-header.c
+++ b/gobex/gobex-header.c
@@ -62,6 +62,34 @@  static glong utf8_to_utf16(gunichar2 **utf16, const char *utf8) {
 	return utf16_len;
 }
 
+static glong utf16_to_utf8(char **utf8, const gunichar2 *utf16, guint16 len,
+				GError **err)
+{
+	glong utf8_len;
+	guint16 utf16_len, i;
+	gunichar2 *buf;
+
+	if (*utf16 == '\0') {
+		*utf8 = NULL;
+		return 0;
+	}
+
+	/* OBEX requires network byteorder (big endian) UTF-16
+	 * but g_utf16_to_utf8 expects host-byteorder UTF-8
+	 */
+	utf16_len = len / sizeof(gunichar2);
+	buf = alloca(sizeof(gunichar2) * utf16_len);
+	for (i = 0; i < utf16_len; i++)
+		(buf)[i] = g_ntohs(utf16[i]);
+	buf[utf16_len] = '\0';
+
+	*utf8 = g_utf16_to_utf8(buf, -1, NULL, &utf8_len, err);
+	if (*utf8 == NULL)
+		utf8_len = -1;
+
+	return utf8_len;
+}
+
 static guint8 *put_bytes(guint8 *to, const void *from, gsize count)
 {
 	memcpy(to, from, count);
@@ -130,7 +158,7 @@  GObexHeader *g_obex_header_decode(const void *data, gsize len,
 	GObexHeader *header;
 	const guint8 *ptr = data;
 	guint16 hdr_len;
-	gsize str_len;
+	glong str_len;
 	GError *conv_err = NULL;
 
 	if (len < 2) {
@@ -177,13 +205,14 @@  GObexHeader *g_obex_header_decode(const void *data, gsize len,
 			goto failed;
 		}
 
-		header->v.string = g_convert((const char *) ptr, hdr_len - 5,
-						"UTF-8", "UTF-16BE",
-						NULL, &str_len, &conv_err);
-		if (header->v.string == NULL) {
+		str_len = utf16_to_utf8(&header->v.string,
+					(const gunichar2 *) ptr,
+					hdr_len - 5,
+					&conv_err);
+		if (str_len < 0) {
 			g_set_error(err, G_OBEX_ERROR,
 					G_OBEX_ERROR_PARSE_ERROR,
-					"Unicode conversion failed: %s",
+					"UTF16 to UTF8 conversion failed: %s",
 					conv_err->message);
 			g_error_free(conv_err);
 			goto failed;