[libvirt PATCH 0/3] Remove usage of virHexToBin (glib chronicles)
Prefer g_ascii_xdigit_value Ján Tomko (3): util: uuid: remove use of virHexToBin Remove all use of virHexToBin util: remove virHexToBin src/libvirt_private.syms | 1 - src/util/virbitmap.c | 3 +-- src/util/virmacaddr.c | 5 ++--- src/util/virutil.c | 15 --------------- src/util/virutil.h | 2 -- src/util/viruuid.c | 11 +++++------ src/vmx/vmx.c | 4 ++-- 7 files changed, 10 insertions(+), 31 deletions(-) -- 2.24.1
Prefer g_ascii_xdigit_value to virHexToBin. Check the return value of the function and remove the g_ascii_isxdigit calls, since they're done anyway internally. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/viruuid.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/util/viruuid.c b/src/util/viruuid.c index c8ee59beee..908b09945d 100644 --- a/src/util/viruuid.c +++ b/src/util/viruuid.c @@ -29,7 +29,6 @@ #include <unistd.h> #include "internal.h" -#include "virutil.h" #include "virerror.h" #include "virlog.h" #include "viralloc.h" @@ -105,6 +104,7 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) cur++; for (i = 0; i < VIR_UUID_BUFLEN;) { + int val; uuid[i] = 0; if (*cur == 0) return -1; @@ -112,16 +112,15 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) cur++; continue; } - if (!g_ascii_isxdigit(*cur)) + if ((val = g_ascii_xdigit_value(*cur)) < 0) return -1; - uuid[i] = virHexToBin(*cur); - uuid[i] *= 16; + uuid[i] = 16 * val; cur++; if (*cur == 0) return -1; - if (!g_ascii_isxdigit(*cur)) + if ((val = g_ascii_xdigit_value(*cur)) < 0) return -1; - uuid[i] += virHexToBin(*cur); + uuid[i] += val; i++; cur++; } -- 2.24.1
On 2/23/20 3:20 PM, Ján Tomko wrote:
Prefer g_ascii_xdigit_value to virHexToBin.
Check the return value of the function and remove the g_ascii_isxdigit calls, since they're done anyway internally.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com> (The two functions behave differently in the case of being passed invalid hex-ascii characters, but all our uses check for valid input prior to calling, so that difference doesn't matter).
--- src/util/viruuid.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/util/viruuid.c b/src/util/viruuid.c index c8ee59beee..908b09945d 100644 --- a/src/util/viruuid.c +++ b/src/util/viruuid.c @@ -29,7 +29,6 @@ #include <unistd.h>
#include "internal.h" -#include "virutil.h" #include "virerror.h" #include "virlog.h" #include "viralloc.h" @@ -105,6 +104,7 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) cur++;
for (i = 0; i < VIR_UUID_BUFLEN;) { + int val; uuid[i] = 0; if (*cur == 0) return -1; @@ -112,16 +112,15 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) cur++; continue; } - if (!g_ascii_isxdigit(*cur)) + if ((val = g_ascii_xdigit_value(*cur)) < 0) return -1; - uuid[i] = virHexToBin(*cur); - uuid[i] *= 16; + uuid[i] = 16 * val; cur++; if (*cur == 0) return -1; - if (!g_ascii_isxdigit(*cur)) + if ((val = g_ascii_xdigit_value(*cur)) < 0) return -1; - uuid[i] += virHexToBin(*cur); + uuid[i] += val; i++; cur++; }
Replace it by g_ascii_xdigit_value. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virbitmap.c | 3 +-- src/util/virmacaddr.c | 5 ++--- src/vmx/vmx.c | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 0679915f70..baf77fe556 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -27,7 +27,6 @@ #include "viralloc.h" #include "virbuffer.h" #include "virstring.h" -#include "virutil.h" #include "virerror.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -1168,7 +1167,7 @@ virBitmapNewString(const char *string) return NULL; for (i = 0; i < len; i++) { - unsigned long nibble = virHexToBin(string[len - i - 1]); + unsigned long nibble = g_ascii_xdigit_value(string[len - i - 1]); nibble <<= VIR_BITMAP_BIT_OFFSET(i * 4); bitmap->map[VIR_BITMAP_UNIT_OFFSET(i * 4)] |= nibble; } diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c index 182bd582fb..35918bca41 100644 --- a/src/util/virmacaddr.c +++ b/src/util/virmacaddr.c @@ -23,7 +23,6 @@ #include "virmacaddr.h" #include "virrandom.h" -#include "virutil.h" #include "viralloc.h" static const unsigned char virMacAddrBroadcastAddrRaw[VIR_MAC_BUFLEN] = @@ -213,8 +212,8 @@ virMacAddrParseHex(const char *str, virMacAddrPtr addr) return -1; for (i = 0; i < VIR_MAC_BUFLEN; i++) - addr->addr[i] = (virHexToBin(str[2 * i]) << 4 | - virHexToBin(str[2 * i + 1])); + addr->addr[i] = (g_ascii_xdigit_value(str[2 * i]) << 4 | + g_ascii_xdigit_value(str[2 * i + 1])); return 0; } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 4362da6cee..b0f7b6a977 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -28,7 +28,6 @@ #include "virconf.h" #include "viralloc.h" #include "virlog.h" -#include "viruuid.h" #include "vmx.h" #include "viruri.h" #include "virstring.h" @@ -673,7 +672,8 @@ virVMXUnescapeHex(char *string, char escape) if (!g_ascii_isxdigit(tmp1[1]) || !g_ascii_isxdigit(tmp1[2])) return -1; - *tmp2++ = virHexToBin(tmp1[1]) * 16 + virHexToBin(tmp1[2]); + *tmp2++ = g_ascii_xdigit_value(tmp1[1]) * 16 + + g_ascii_xdigit_value(tmp1[2]); tmp1 += 3; } else { *tmp2++ = *tmp1++; -- 2.24.1
Now that it is no longer used. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virutil.c | 15 --------------- src/util/virutil.h | 2 -- 3 files changed, 18 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 907cef2390..b268142d67 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3408,7 +3408,6 @@ virGetUserID; virGetUserName; virGetUserRuntimeDirectory; virGetUserShell; -virHexToBin; virHostGetDRMRenderNode; virHostHasIOMMU; virIndexToDiskName; diff --git a/src/util/virutil.c b/src/util/virutil.c index bdb79c7f1b..f77ee05dbf 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -164,21 +164,6 @@ int virSetSockReuseAddr(int fd, bool fatal) #endif -/* Convert C from hexadecimal character to integer. */ -int -virHexToBin(unsigned char c) -{ - switch (c) { - default: return c - '0'; - case 'a': case 'A': return 10; - case 'b': case 'B': return 11; - case 'c': case 'C': return 12; - case 'd': case 'D': return 13; - case 'e': case 'E': return 14; - case 'f': case 'F': return 15; - } -} - /* Scale an integer VALUE in-place by an optional case-insensitive * SUFFIX, defaulting to SCALE if suffix is NULL or empty (scale is * typically 1 or 1024). Recognized suffixes include 'b' or 'bytes', diff --git a/src/util/virutil.h b/src/util/virutil.h index 7377c8c8da..cc6b82a177 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -44,8 +44,6 @@ int virScaleInteger(unsigned long long *value, const char *suffix, unsigned long long scale, unsigned long long limit) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -int virHexToBin(unsigned char c); - int virParseVersionString(const char *str, unsigned long *version, bool allowMissing); -- 2.24.1
On 2/23/20 3:20 PM, Ján Tomko wrote:
Prefer g_ascii_xdigit_value
Ján Tomko (3): util: uuid: remove use of virHexToBin Remove all use of virHexToBin util: remove virHexToBin
src/libvirt_private.syms | 1 - src/util/virbitmap.c | 3 +-- src/util/virmacaddr.c | 5 ++--- src/util/virutil.c | 15 --------------- src/util/virutil.h | 2 -- src/util/viruuid.c | 11 +++++------ src/vmx/vmx.c | 4 ++-- 7 files changed, 10 insertions(+), 31 deletions(-)
Hmmpph. I meant to respond to 0/3 with my Reviewed-by: Laine Stump <laine@redhat.com> not just to the first patch.
participants (3)
-
Ján Tomko -
Laine Stump -
Laine Stump