1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE

Peter Krempa posted 1 patch 4 years, 6 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/88e207cf093e9dad9d10899d6dce8e5729fd17e4.1571926064.git.pkrempa@redhat.com
src/internal.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
[libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE
Posted by Peter Krempa 4 years, 6 months ago 
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/internal.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index 5b0a2335f5..0ff9f496ac 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -86,10 +86,8 @@
 #define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0)
 #define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL)

-#define STREQ_NULLABLE(a, b) \
-    ((a) ? (b) && STREQ((a), (b)) : !(b))
-#define STRNEQ_NULLABLE(a, b) \
-    ((a) ? !(b) || STRNEQ((a), (b)) : !!(b))
+#define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0)
+#define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0)

 #define NUL_TERMINATE(buf) do { (buf)[sizeof(buf)-1] = '\0'; } while (0)

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE
Posted by Ján Tomko 4 years, 6 months ago 
On Thu, Oct 24, 2019 at 04:12:00PM +0200, Peter Krempa wrote:
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/internal.h | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE
Posted by Pavel Hrdina 4 years, 6 months ago 
On Thu, Oct 24, 2019 at 04:12:00PM +0200, Peter Krempa wrote:
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/internal.h | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/internal.h b/src/internal.h
> index 5b0a2335f5..0ff9f496ac 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -86,10 +86,8 @@
>  #define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0)
>  #define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL)
> 
> -#define STREQ_NULLABLE(a, b) \
> -    ((a) ? (b) && STREQ((a), (b)) : !(b))
> -#define STRNEQ_NULLABLE(a, b) \
> -    ((a) ? !(b) || STRNEQ((a), (b)) : !!(b))
> +#define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0)
> +#define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0)

Why not use g_strcmp0 directly?

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE
Posted by Daniel P. Berrangé 4 years, 6 months ago 
On Thu, Oct 24, 2019 at 04:22:09PM +0200, Pavel Hrdina wrote:
> On Thu, Oct 24, 2019 at 04:12:00PM +0200, Peter Krempa wrote:
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/internal.h | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/internal.h b/src/internal.h
> > index 5b0a2335f5..0ff9f496ac 100644
> > --- a/src/internal.h
> > +++ b/src/internal.h
> > @@ -86,10 +86,8 @@
> >  #define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0)
> >  #define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL)
> > 
> > -#define STREQ_NULLABLE(a, b) \
> > -    ((a) ? (b) && STREQ((a), (b)) : !(b))
> > -#define STRNEQ_NULLABLE(a, b) \
> > -    ((a) ? !(b) || STRNEQ((a), (b)) : !!(b))
> > +#define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0)
> > +#define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0)
> 
> Why not use g_strcmp0 directly?

The same reason we never used  strcmp() in the first place - it is
easier to read STREQ vs  strcmp == 0 / strcmp != 0.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE
Posted by Pavel Hrdina 4 years, 6 months ago 
On Thu, Oct 24, 2019 at 03:29:27PM +0100, Daniel P. Berrangé wrote:
> On Thu, Oct 24, 2019 at 04:22:09PM +0200, Pavel Hrdina wrote:
> > On Thu, Oct 24, 2019 at 04:12:00PM +0200, Peter Krempa wrote:
> > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > ---
> > >  src/internal.h | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/internal.h b/src/internal.h
> > > index 5b0a2335f5..0ff9f496ac 100644
> > > --- a/src/internal.h
> > > +++ b/src/internal.h
> > > @@ -86,10 +86,8 @@
> > >  #define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0)
> > >  #define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL)
> > > 
> > > -#define STREQ_NULLABLE(a, b) \
> > > -    ((a) ? (b) && STREQ((a), (b)) : !(b))
> > > -#define STRNEQ_NULLABLE(a, b) \
> > > -    ((a) ? !(b) || STRNEQ((a), (b)) : !!(b))
> > > +#define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0)
> > > +#define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0)
> > 
> > Why not use g_strcmp0 directly?
> 
> The same reason we never used  strcmp() in the first place - it is
> easier to read STREQ vs  strcmp == 0 / strcmp != 0.

Right, I was looking at g_str_equal() which returns gboolean and for
some reason didn't realized that g_strcmp0 returns int.  Only if there
would be g_str_equal0().

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE
Posted by Peter Krempa 4 years, 6 months ago 
On Thu, Oct 24, 2019 at 16:22:09 +0200, Pavel Hrdina wrote:
> On Thu, Oct 24, 2019 at 04:12:00PM +0200, Peter Krempa wrote:
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/internal.h | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/internal.h b/src/internal.h
> > index 5b0a2335f5..0ff9f496ac 100644
> > --- a/src/internal.h
> > +++ b/src/internal.h
> > @@ -86,10 +86,8 @@
> >  #define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0)
> >  #define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL)
> > 
> > -#define STREQ_NULLABLE(a, b) \
> > -    ((a) ? (b) && STREQ((a), (b)) : !(b))
> > -#define STRNEQ_NULLABLE(a, b) \
> > -    ((a) ? !(b) || STRNEQ((a), (b)) : !!(b))
> > +#define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0)
> > +#define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0)
> 
> Why not use g_strcmp0 directly?

Because I don't really care about replacing everyting.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE
Posted by Laine Stump 4 years, 6 months ago 
On 10/24/19 10:31 AM, Peter Krempa wrote:
> On Thu, Oct 24, 2019 at 16:22:09 +0200, Pavel Hrdina wrote:
>> On Thu, Oct 24, 2019 at 04:12:00PM +0200, Peter Krempa wrote:
>>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>>> ---
>>>   src/internal.h | 6 ++----
>>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/internal.h b/src/internal.h
>>> index 5b0a2335f5..0ff9f496ac 100644
>>> --- a/src/internal.h
>>> +++ b/src/internal.h
>>> @@ -86,10 +86,8 @@
>>>   #define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0)
>>>   #define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL)
>>>
>>> -#define STREQ_NULLABLE(a, b) \
>>> -    ((a) ? (b) && STREQ((a), (b)) : !(b))
>>> -#define STRNEQ_NULLABLE(a, b) \
>>> -    ((a) ? !(b) || STRNEQ((a), (b)) : !!(b))
>>> +#define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0)
>>> +#define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0)
>> Why not use g_strcmp0 directly?
> Because I don't really care about replacing everyting.


Yeah, on one hand using the glib functions directly might make it easier 
for someone familiar with glib to understand the code when just glancing 
at it. On the other hand, replacing all these macros with direct calls 
to glib seems like it's undoing a lot of work, and creating 
code-churn/obscuring true "git blame", and will just need to be done 
again in 2029 when glib is no longer the flavor of the day, and we want 
to convert everything to use "hlib" or "ilib" (or maybe by then it will 
be "postapocalypticlib" - think about it, having all our code sprinkled 
with postapocalyptic_dystopian_strcmp0() will be *so* future proof!)


(The preceding paragraph was only half sarcastic, same for this sentence.)


>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.