[BlueZ] gobex: Replace g_convert by utf16_to_utf8

Commit Message

Frédéric Danis Sept. 3, 2024, 4:11 p.m. UTC

g_lib may be built without the gconv modules for UTF-16, which
prevent to use g_convert().
This commit replaces it by a function similar to the existing
utf8_to_utf16() function.
---
 gobex/gobex-header.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

Comments

Luiz Augusto von Dentz Sept. 3, 2024, 4:33 p.m. UTC | #1

Hi Frédéric,

On Tue, Sep 3, 2024 at 12:11 PM Frédéric Danis
<[e-mail]> wrote:
>
> g_lib may be built without the gconv modules for UTF-16, which
> prevent to use g_convert().
> This commit replaces it by a function similar to the existing
> utf8_to_utf16() function.
> ---
>  gobex/gobex-header.c | 39 +++++++++++++++++++++++++++++++++------
>  1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/gobex/gobex-header.c b/gobex/gobex-header.c
> index 002ba8861..b43374da6 100644
> --- a/gobex/gobex-header.c
> +++ b/gobex/gobex-header.c
> @@ -62,6 +62,32 @@ static glong utf8_to_utf16(gunichar2 **utf16, const char *utf8) {
>         return utf16_len;
>  }
>
> +static glong utf16_to_utf8(char **utf8, const gunichar2 *utf16, glong len,
> +                               GError **err)
> +{
> +       glong utf8_len;
> +       int 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
> +        */
> +       buf = g_malloc0(sizeof(gunichar2) * len);
> +       for (i = 0; i < len; i++)
> +               (buf)[i] = g_ntohs(utf16[i]);

Shouldn't the buf variable be freed before going out of scope? Or is
it reused as return by g_utf16_to_utf8? Perhaps we could use a stack
variable instead?

> +       *utf8 = g_utf16_to_utf8(buf, -1, NULL, &utf8_len, err);
> +       if (*utf8 == NULL)
> +               return -1;
> +
> +       return utf8_len;
> +}
> +
>  static guint8 *put_bytes(guint8 *to, const void *from, gsize count)
>  {
>         memcpy(to, from, count);
> @@ -130,7 +156,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 +203,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
>
>

[e-mail] Sept. 3, 2024, 5:47 p.m. UTC | #2

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=886383

---Test result---

Test Summary:
CheckPatch                    PASS      0.51 seconds
GitLint                       PASS      0.32 seconds
BuildEll                      PASS      24.55 seconds
BluezMake                     PASS      1713.07 seconds
MakeCheck                     FAIL      13.81 seconds
MakeDistcheck                 PASS      180.57 seconds
CheckValgrind                 FAIL      252.29 seconds
CheckSmatch                   PASS      358.24 seconds
bluezmakeextell               PASS      120.05 seconds
IncrementalBuild              PASS      1504.19 seconds
ScanBuild                     WARNING   1005.30 seconds

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

make[3]: *** [Makefile:11766: test-suite.log] Error 1
make[2]: *** [Makefile:11874: check-TESTS] Error 2
make[1]: *** [Makefile:12303: check-am] Error 2
make: *** [Makefile:12305: check] Error 2
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:

tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12725:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12725 | int main(int argc, char *argv[])
      |     ^~~~
make[3]: *** [Makefile:11766: test-suite.log] Error 1
make[2]: *** [Makefile:11874: check-TESTS] Error 2
make[1]: *** [Makefile:12303: check-am] Error 2
make: *** [Makefile:12305: check] Error 2
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
gobex/gobex-header.c:84:8: warning: Potential leak of memory pointed to by 'buf'
        *utf8 = g_utf16_to_utf8(buf, -1, NULL, &utf8_len, err);
        ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
gobex/gobex-header.c:93:2: warning: Null pointer passed to 2nd parameter expecting 'nonnull'
        memcpy(to, from, count);
        ^~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.



---
Regards,
Linux Bluetooth

Patch

diff --git a/gobex/gobex-header.c b/gobex/gobex-header.c
index 002ba8861..b43374da6 100644
--- a/gobex/gobex-header.c
+++ b/gobex/gobex-header.c
@@ -62,6 +62,32 @@  static glong utf8_to_utf16(gunichar2 **utf16, const char *utf8) {
 	return utf16_len;
 }
 
+static glong utf16_to_utf8(char **utf8, const gunichar2 *utf16, glong len,
+				GError **err)
+{
+	glong utf8_len;
+	int 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
+	 */
+	buf = g_malloc0(sizeof(gunichar2) * len);
+	for (i = 0; i < len; i++)
+		(buf)[i] = g_ntohs(utf16[i]);
+
+	*utf8 = g_utf16_to_utf8(buf, -1, NULL, &utf8_len, err);
+	if (*utf8 == NULL)
+		return -1;
+
+	return utf8_len;
+}
+
 static guint8 *put_bytes(guint8 *to, const void *from, gsize count)
 {
 	memcpy(to, from, count);
@@ -130,7 +156,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 +203,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;