[Qemu-devel] [PATCH 0/5] qga: win32 guest-set-user-password improvements

[Qemu-devel] [PATCH 0/5] qga: win32 guest-set-user-password improvements

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

Laszlo Ersek pointed out a few worthy improvements to the win32
guest-set-user-password command.

Marc-André Lureau (5):
  qga: use more idiomatic qemu-style eol operators
  qga: use size_t for wcslen() return value
  qga: use wide-chars constants for wchar_t comparisons
  qga: fix off-by-one length check
  qga: check utf8-to-utf16 conversion

 qga/commands-win32.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH 1/5] qga: use more idiomatic qemu-style eol operators

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/commands-win32.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index cf0757c..7baacc7 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1248,9 +1248,9 @@ get_net_error_message(gint error)
     wchar_t *msg = NULL;
     int flags, nchars;
 
-    flags = FORMAT_MESSAGE_ALLOCATE_BUFFER
-        |FORMAT_MESSAGE_IGNORE_INSERTS
-        |FORMAT_MESSAGE_FROM_SYSTEM;
+    flags = FORMAT_MESSAGE_ALLOCATE_BUFFER |
+        FORMAT_MESSAGE_IGNORE_INSERTS |
+        FORMAT_MESSAGE_FROM_SYSTEM;
 
     if (error >= NERR_BASE && error <= MAX_NERR) {
         module = LoadLibraryExW(L"netmsg.dll", NULL, LOAD_LIBRARY_AS_DATAFILE);
-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/5] qga: use more idiomatic qemu-style eol operators

[Michael Roth]

Quoting marcandre.lureau@redhat.com (2016-02-17 10:47:51)
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  qga/commands-win32.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index cf0757c..7baacc7 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1248,9 +1248,9 @@ get_net_error_message(gint error)
>      wchar_t *msg = NULL;
>      int flags, nchars;
> 
> -    flags = FORMAT_MESSAGE_ALLOCATE_BUFFER
> -        |FORMAT_MESSAGE_IGNORE_INSERTS
> -        |FORMAT_MESSAGE_FROM_SYSTEM;
> +    flags = FORMAT_MESSAGE_ALLOCATE_BUFFER |
> +        FORMAT_MESSAGE_IGNORE_INSERTS |
> +        FORMAT_MESSAGE_FROM_SYSTEM;
> 
>      if (error >= NERR_BASE && error <= MAX_NERR) {
>          module = LoadLibraryExW(L"netmsg.dll", NULL, LOAD_LIBRARY_AS_DATAFILE);
> -- 
> 2.5.0
> 

[Qemu-devel] [PATCH 2/5] qga: use size_t for wcslen() return value

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/commands-win32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 7baacc7..e5afc5d 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1246,7 +1246,8 @@ get_net_error_message(gint error)
     HMODULE module = NULL;
     gchar *retval = NULL;
     wchar_t *msg = NULL;
-    int flags, nchars;
+    int flags;
+    size_t nchars;
 
     flags = FORMAT_MESSAGE_ALLOCATE_BUFFER |
         FORMAT_MESSAGE_IGNORE_INSERTS |
-- 
2.5.0

Re: [Qemu-devel] [PATCH 2/5] qga: use size_t for wcslen() return value

[Michael Roth]

Quoting marcandre.lureau@redhat.com (2016-02-17 10:47:52)
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  qga/commands-win32.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 7baacc7..e5afc5d 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1246,7 +1246,8 @@ get_net_error_message(gint error)
>      HMODULE module = NULL;
>      gchar *retval = NULL;
>      wchar_t *msg = NULL;
> -    int flags, nchars;
> +    int flags;
> +    size_t nchars;
> 
>      flags = FORMAT_MESSAGE_ALLOCATE_BUFFER |
>          FORMAT_MESSAGE_IGNORE_INSERTS |
> -- 
> 2.5.0
> 

[Qemu-devel] [PATCH 3/5] qga: use wide-chars constants for wchar_t comparisons

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/commands-win32.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index e5afc5d..b20f107 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1266,8 +1266,10 @@ get_net_error_message(gint error)
     if (msg != NULL) {
         nchars = wcslen(msg);
 
-        if (nchars > 2 && msg[nchars-1] == '\n' && msg[nchars-2] == '\r') {
-            msg[nchars-2] = '\0';
+        if (nchars > 2 &&
+            msg[nchars - 1] == L'\n' &&
+            msg[nchars - 2] == L'\r') {
+            msg[nchars - 2] = L'\0';
         }
 
         retval = g_utf16_to_utf8(msg, -1, NULL, NULL, NULL);
-- 
2.5.0

Re: [Qemu-devel] [PATCH 3/5] qga: use wide-chars constants for wchar_t comparisons

[Michael Roth]

Quoting marcandre.lureau@redhat.com (2016-02-17 10:47:53)
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  qga/commands-win32.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index e5afc5d..b20f107 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1266,8 +1266,10 @@ get_net_error_message(gint error)
>      if (msg != NULL) {
>          nchars = wcslen(msg);
> 
> -        if (nchars > 2 && msg[nchars-1] == '\n' && msg[nchars-2] == '\r') {
> -            msg[nchars-2] = '\0';
> +        if (nchars > 2 &&
> +            msg[nchars - 1] == L'\n' &&
> +            msg[nchars - 2] == L'\r') {
> +            msg[nchars - 2] = L'\0';
>          }
> 
>          retval = g_utf16_to_utf8(msg, -1, NULL, NULL, NULL);
> -- 
> 2.5.0
> 

[Qemu-devel] [PATCH 4/5] qga: fix off-by-one length check

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Laszlo Ersek said: "The length check is off by one (in the safe direction); it
should be (nchars >= 2). The processing should be active for the wide string
L"\r\n" -- resulting in the empty wide string --, I believe."

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/commands-win32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index b20f107..ae8cf3d 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1266,7 +1266,7 @@ get_net_error_message(gint error)
     if (msg != NULL) {
         nchars = wcslen(msg);
 
-        if (nchars > 2 &&
+        if (nchars >= 2 &&
             msg[nchars - 1] == L'\n' &&
             msg[nchars - 2] == L'\r') {
             msg[nchars - 2] = L'\0';
-- 
2.5.0

Re: [Qemu-devel] [PATCH 4/5] qga: fix off-by-one length check

[Michael Roth]

Quoting marcandre.lureau@redhat.com (2016-02-17 10:47:54)
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Laszlo Ersek said: "The length check is off by one (in the safe direction); it
> should be (nchars >= 2). The processing should be active for the wide string
> L"\r\n" -- resulting in the empty wide string --, I believe."
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  qga/commands-win32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index b20f107..ae8cf3d 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1266,7 +1266,7 @@ get_net_error_message(gint error)
>      if (msg != NULL) {
>          nchars = wcslen(msg);
> 
> -        if (nchars > 2 &&
> +        if (nchars >= 2 &&
>              msg[nchars - 1] == L'\n' &&
>              msg[nchars - 2] == L'\r') {
>              msg[nchars - 2] = L'\0';
> -- 
> 2.5.0
> 

[Qemu-devel] [PATCH 5/5] qga: check utf8-to-utf16 conversion

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

UTF8 to UTF16 conversion can fail for genuine reasons, let's check errors.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/commands-win32.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index ae8cf3d..09ed465 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1292,8 +1292,9 @@ void qmp_guest_set_user_password(const char *username,
     NET_API_STATUS nas;
     char *rawpasswddata = NULL;
     size_t rawpasswdlen;
-    wchar_t *user, *wpass;
+    wchar_t *user = NULL, *wpass = NULL;
     USER_INFO_1003 pi1003 = { 0, };
+    GError *gerr = NULL;
 
     if (crypted) {
         error_setg(errp, QERR_UNSUPPORTED);
@@ -1307,8 +1308,15 @@ void qmp_guest_set_user_password(const char *username,
     rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
     rawpasswddata[rawpasswdlen] = '\0';
 
-    user = g_utf8_to_utf16(username, -1, NULL, NULL, NULL);
-    wpass = g_utf8_to_utf16(rawpasswddata, -1, NULL, NULL, NULL);
+    user = g_utf8_to_utf16(username, -1, NULL, NULL, &gerr);
+    if (!user) {
+        goto done;
+    }
+
+    wpass = g_utf8_to_utf16(rawpasswddata, -1, NULL, NULL, &gerr);
+    if (!wpass) {
+        goto done;
+    }
 
     pi1003.usri1003_password = wpass;
     nas = NetUserSetInfo(NULL, user,
@@ -1321,6 +1329,11 @@ void qmp_guest_set_user_password(const char *username,
         g_free(msg);
     }
 
+done:
+    if (gerr) {
+        error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message);
+        g_error_free(gerr);
+    }
     g_free(user);
     g_free(wpass);
     g_free(rawpasswddata);
-- 
2.5.0

Re: [Qemu-devel] [PATCH 5/5] qga: check utf8-to-utf16 conversion

[Michael Roth]

Quoting marcandre.lureau@redhat.com (2016-02-17 10:47:55)
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> UTF8 to UTF16 conversion can fail for genuine reasons, let's check errors.
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  qga/commands-win32.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index ae8cf3d..09ed465 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1292,8 +1292,9 @@ void qmp_guest_set_user_password(const char *username,
>      NET_API_STATUS nas;
>      char *rawpasswddata = NULL;
>      size_t rawpasswdlen;
> -    wchar_t *user, *wpass;
> +    wchar_t *user = NULL, *wpass = NULL;
>      USER_INFO_1003 pi1003 = { 0, };
> +    GError *gerr = NULL;
> 
>      if (crypted) {
>          error_setg(errp, QERR_UNSUPPORTED);
> @@ -1307,8 +1308,15 @@ void qmp_guest_set_user_password(const char *username,
>      rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
>      rawpasswddata[rawpasswdlen] = '\0';
> 
> -    user = g_utf8_to_utf16(username, -1, NULL, NULL, NULL);
> -    wpass = g_utf8_to_utf16(rawpasswddata, -1, NULL, NULL, NULL);
> +    user = g_utf8_to_utf16(username, -1, NULL, NULL, &gerr);
> +    if (!user) {
> +        goto done;
> +    }
> +
> +    wpass = g_utf8_to_utf16(rawpasswddata, -1, NULL, NULL, &gerr);
> +    if (!wpass) {
> +        goto done;
> +    }
> 
>      pi1003.usri1003_password = wpass;
>      nas = NetUserSetInfo(NULL, user,
> @@ -1321,6 +1329,11 @@ void qmp_guest_set_user_password(const char *username,
>          g_free(msg);
>      }
> 
> +done:
> +    if (gerr) {
> +        error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message);
> +        g_error_free(gerr);
> +    }
>      g_free(user);
>      g_free(wpass);
>      g_free(rawpasswddata);
> -- 
> 2.5.0
> 

Re: [Qemu-devel] [PATCH 0/5] qga: win32 guest-set-user-password improvements

[Laszlo Ersek]

On 02/17/16 17:47, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> Laszlo Ersek pointed out a few worthy improvements to the win32
> guest-set-user-password command.
> 
> Marc-André Lureau (5):
>   qga: use more idiomatic qemu-style eol operators
>   qga: use size_t for wcslen() return value
>   qga: use wide-chars constants for wchar_t comparisons
>   qga: fix off-by-one length check
>   qga: check utf8-to-utf16 conversion
> 
>  qga/commands-win32.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 

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

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH 0/5] qga: win32 guest-set-user-password improvements

[Michael Roth]

Quoting marcandre.lureau@redhat.com (2016-02-17 10:47:50)
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> Laszlo Ersek pointed out a few worthy improvements to the win32
> guest-set-user-password command.
> 
> Marc-André Lureau (5):
>   qga: use more idiomatic qemu-style eol operators
>   qga: use size_t for wcslen() return value
>   qga: use wide-chars constants for wchar_t comparisons
>   qga: fix off-by-one length check
>   qga: check utf8-to-utf16 conversion

Thanks, applied to qga tree:

  https://github.com/mdroth/qemu/commits/qga

> 
>  qga/commands-win32.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> -- 
> 2.5.0
>