1. Patchew
  2. Libvirt
  3. View series

[PATCH] REFACTOR: Eliminate newlines in messages and fixes a typo

K Shiva Kiran posted 1 patch 8 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230825152938.6481-1-shiva._5Fkr@riseup.net
include/libvirt/libvirt-domain.h | 2 +-
src/libvirt-domain.c             | 3 +--
src/libvirt-network.c            | 3 +--
tests/metadatatest.c             | 9 +++------
tests/networkmetadatatest.c      | 9 +++------
5 files changed, 9 insertions(+), 17 deletions(-)
[PATCH] REFACTOR: Eliminate newlines in messages and fixes a typo
Posted by K Shiva Kiran 8 months, 1 week ago 
Removes newlines of error message strings in metadata unit tests,
libvirt-domain.c and libvirt-network.c
Fixes a minor typo in libvirt-domain.c

Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net>
---
 include/libvirt/libvirt-domain.h | 2 +-
 src/libvirt-domain.c             | 3 +--
 src/libvirt-network.c            | 3 +--
 tests/metadatatest.c             | 9 +++------
 tests/networkmetadatatest.c      | 9 +++------
 5 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index a1902546bb..ea36805aa3 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5184,7 +5184,7 @@ typedef void (*virConnectDomainEventDeviceRemovalFailedCallback)(virConnectPtr c
  * virConnectDomainEventMetadataChangeCallback:
  * @conn: connection object
  * @dom: domain on which the event occurred
- * @type: a value from virDomainMetadataTypea
+ * @type: a value from virDomainMetadataType
  * @nsuri: XML namespace URI
  * @opaque: application specified data
  *
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index ec42bb9a53..80a554a530 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8585,8 +8585,7 @@ virDomainSetMetadata(virDomainPtr domain,
     case VIR_DOMAIN_METADATA_TITLE:
         if (metadata && strchr(metadata, '\n')) {
             virReportInvalidArg(metadata, "%s",
-                                _("metadata title can't contain "
-                                  "newlines"));
+                                _("metadata title can't contain newlines"));
             goto error;
         }
         G_GNUC_FALLTHROUGH;
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index c0daea3a60..ef17a8a04d 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -1974,8 +1974,7 @@ virNetworkSetMetadata(virNetworkPtr network,
     case VIR_NETWORK_METADATA_TITLE:
         if (metadata && strchr(metadata, '\n')) {
             virReportInvalidArg(metadata, "%s",
-                                _("metadata title can't contain "
-                                  "newlines"));
+                                _("metadata title can't contain newlines"));
             goto error;
         }
         G_GNUC_FALLTHROUGH;
diff --git a/tests/metadatatest.c b/tests/metadatatest.c
index b56428fde5..7136730e6a 100644
--- a/tests/metadatatest.c
+++ b/tests/metadatatest.c
@@ -113,8 +113,7 @@ verifyMetadata(virDomainPtr dom,
 
         if (STRNEQ(metadataAPI, expectAPI)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
-                           "XML metadata in API doesn't match expected metadata: "
-                           "expected:\n[%s]\ngot:\n[%s]",
+                           "XML metadata in API doesn't match expected metadata: expected:\n[%s]\ngot:\n[%s]",
                            expectAPI, metadataAPI);
             return false;
         }
@@ -136,8 +135,7 @@ verifyMetadata(virDomainPtr dom,
 
         if (STRNEQ(metadataXML, expectXML)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
-                           "XML in dump doesn't match expected metadata: "
-                           "expected:\n[%s]\ngot:\n[%s]",
+                           "XML in dump doesn't match expected metadata: expected:\n[%s]\ngot:\n[%s]",
                            expectXML, metadataXML);
             return false;
         }
@@ -219,8 +217,7 @@ testTextMetadata(const void *data)
 
     if (STRNEQ_NULLABLE(test->expect, actual)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "expected metadata doesn't match actual: "
-                       "expected:'%s'\ngot: '%s'",
+                       "expected metadata doesn't match actual: expected:'%s'\ngot: '%s'",
                        NULLSTR(test->data), NULLSTR(actual));
         return -1;
     }
diff --git a/tests/networkmetadatatest.c b/tests/networkmetadatatest.c
index c309fb29b0..7a1bca4f73 100644
--- a/tests/networkmetadatatest.c
+++ b/tests/networkmetadatatest.c
@@ -113,8 +113,7 @@ verifyMetadata(virNetworkPtr net,
 
         if (STRNEQ(metadataAPI, expectAPI)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
-                           "XML metadata in API doesn't match expected metadata: "
-                           "expected:\n[%s]\ngot:\n[%s]",
+                           "XML metadata in API doesn't match expected metadata: expected:\n[%s]\ngot:\n[%s]",
                            expectAPI, metadataAPI);
             return false;
         }
@@ -136,8 +135,7 @@ verifyMetadata(virNetworkPtr net,
 
         if (STRNEQ(metadataXML, expectXML)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
-                           "XML in dump doesn't match expected metadata: "
-                           "expected:\n[%s]\ngot:\n[%s]",
+                           "XML in dump doesn't match expected metadata: expected:\n[%s]\ngot:\n[%s]",
                            expectXML, metadataXML);
             return false;
         }
@@ -219,8 +217,7 @@ testTextMetadata(const void *data)
 
     if (STRNEQ_NULLABLE(test->expect, actual)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "expected metadata doesn't match actual: "
-                       "expected:'%s'\ngot: '%s'",
+                       "expected metadata doesn't match actual: expected:'%s'\ngot: '%s'",
                        NULLSTR(test->data), NULLSTR(actual));
         return -1;
     }
-- 
2.42.0
Re: [PATCH] REFACTOR: Eliminate newlines in messages and fixes a typo
Posted by Michal Prívozník 8 months, 1 week ago 
On 8/25/23 17:29, K Shiva Kiran wrote:
> Removes newlines of error message strings in metadata unit tests,
> libvirt-domain.c and libvirt-network.c
> Fixes a minor typo in libvirt-domain.c
> 
> Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net>
> ---
>  include/libvirt/libvirt-domain.h | 2 +-
>  src/libvirt-domain.c             | 3 +--
>  src/libvirt-network.c            | 3 +--
>  tests/metadatatest.c             | 9 +++------
>  tests/networkmetadatatest.c      | 9 +++------
>  5 files changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index a1902546bb..ea36805aa3 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -5184,7 +5184,7 @@ typedef void (*virConnectDomainEventDeviceRemovalFailedCallback)(virConnectPtr c
>   * virConnectDomainEventMetadataChangeCallback:
>   * @conn: connection object
>   * @dom: domain on which the event occurred
> - * @type: a value from virDomainMetadataTypea
> + * @type: a value from virDomainMetadataType
>   * @nsuri: XML namespace URI
>   * @opaque: application specified data
>   *

ACK to this hunk.

> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index ec42bb9a53..80a554a530 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -8585,8 +8585,7 @@ virDomainSetMetadata(virDomainPtr domain,
>      case VIR_DOMAIN_METADATA_TITLE:
>          if (metadata && strchr(metadata, '\n')) {
>              virReportInvalidArg(metadata, "%s",
> -                                _("metadata title can't contain "
> -                                  "newlines"));
> +                                _("metadata title can't contain newlines"));
>              goto error;
>          }
>          G_GNUC_FALLTHROUGH;
> diff --git a/src/libvirt-network.c b/src/libvirt-network.c
> index c0daea3a60..ef17a8a04d 100644
> --- a/src/libvirt-network.c
> +++ b/src/libvirt-network.c
> @@ -1974,8 +1974,7 @@ virNetworkSetMetadata(virNetworkPtr network,
>      case VIR_NETWORK_METADATA_TITLE:
>          if (metadata && strchr(metadata, '\n')) {
>              virReportInvalidArg(metadata, "%s",
> -                                _("metadata title can't contain "
> -                                  "newlines"));
> +                                _("metadata title can't contain newlines"));
>              goto error;
>          }
>          G_GNUC_FALLTHROUGH;


These two - I've already covered in my other series, that's just waiting
for the release to be pushed:

https://listman.redhat.com/archives/libvir-list/2023-August/241416.html

> diff --git a/tests/metadatatest.c b/tests/metadatatest.c
> index b56428fde5..7136730e6a 100644
> --- a/tests/metadatatest.c
> +++ b/tests/metadatatest.c
> @@ -113,8 +113,7 @@ verifyMetadata(virDomainPtr dom,
>  
>          if (STRNEQ(metadataAPI, expectAPI)) {
>              virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           "XML metadata in API doesn't match expected metadata: "
> -                           "expected:\n[%s]\ngot:\n[%s]",
> +                           "XML metadata in API doesn't match expected metadata: expected:\n[%s]\ngot:\n[%s]",
>                             expectAPI, metadataAPI);
>              return false;
>          }

NACK to this one and the rest. If message contains '\n' character then
it's okay if it is on multiple lines. The sole reason we want error
messages on one line is: whenever I see a bug report I can select
"random" substring of the error message and pass it to 'git grep'. If a
message is split on multiple lines though I have to play guessing game
and try to find the exact point where the message was split.

For instance, in the previous two hunks, I can do (after my series is
merged):

git grep "metadata title can't contain newlines"
git grep "metadata title can't contain"
git grep "can't contain newlines"

and all would match the same lines. But if I would do that now, the last
one would match nothing.

But with error messages that contain '\n' it's obviously wrong if I'd
pick a substring containing '\n' and pass it to git grep.

Hence, this hunk and the rest is wrong and make code worse. Sorry.

Michal

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.