[PATCH 03/13] Decrease scope of some variables

Michal Privoznik posted 13 patches 2 years, 6 months ago
[PATCH 03/13] Decrease scope of some variables
Posted by Michal Privoznik 2 years, 6 months ago
There are couple of variables that are declared at function
beginning but then used solely within a block (either for() loop
or if() statement). And just before their use they are zeroed
explicitly using memset(). Decrease their scope, use struct zero
initializer and drop explicit memset().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_driver.c       | 4 +---
 src/remote/remote_driver.c   | 7 +++----
 src/rpc/virnetsshsession.c   | 5 ++---
 src/util/virnetdevmacvlan.c  | 3 +--
 src/util/virnetdevtap.c      | 6 ++----
 src/util/viruuid.c           | 4 ++--
 tests/virnetsockettest.c     | 7 ++-----
 tools/virsh-domain-monitor.c | 7 ++-----
 8 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 497923ffee..1d7b78d73b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17124,7 +17124,6 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED,
                             unsigned int privflags G_GNUC_UNUSED)
 {
     size_t i;
-    struct _virDomainInterfaceStats tmp;
 
     if (!virDomainObjIsActive(dom))
         return 0;
@@ -17135,12 +17134,11 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED,
     for (i = 0; i < dom->def->nnets; i++) {
         virDomainNetDef *net = dom->def->nets[i];
         virDomainNetType actualType;
+        struct _virDomainInterfaceStats tmp = { 0 };
 
         if (!net->ifname)
             continue;
 
-        memset(&tmp, 0, sizeof(tmp));
-
         actualType = virDomainNetGetActualType(net);
 
         virTypedParamListAddString(params, net->ifname, "net.%zu.name", i);
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 65ec239fb7..d775f65fe2 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3728,8 +3728,6 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
     remote_auth_sasl_init_ret iret;
     remote_auth_sasl_start_args sargs = {0};
     remote_auth_sasl_start_ret sret;
-    remote_auth_sasl_step_args pargs = {0};
-    remote_auth_sasl_step_ret pret;
     const char *clientout;
     char *serverin = NULL;
     size_t clientoutlen, serverinlen;
@@ -3866,6 +3864,9 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
      * Even if the server has completed, the client must *always* do at least one step
      * in this loop to verify the server isn't lying about something. Mutual auth */
     for (;;) {
+        remote_auth_sasl_step_args pargs = { 0 };
+        remote_auth_sasl_step_ret pret = { 0 };
+
         if ((err = virNetSASLSessionClientStep(sasl,
                                                serverin,
                                                serverinlen,
@@ -3893,14 +3894,12 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
 
         /* Not done, prepare to talk with the server for another iteration */
         /* NB, distinction of NULL vs "" is *critical* in SASL */
-        memset(&pargs, 0, sizeof(pargs));
         pargs.nil = clientout ? 0 : 1;
         pargs.data.data_val = (char*)clientout;
         pargs.data.data_len = clientoutlen;
         VIR_DEBUG("Server step with %zu bytes %p",
                   clientoutlen, clientout);
 
-        memset(&pret, 0, sizeof(pret));
         if (call(conn, priv, 0, REMOTE_PROC_AUTH_SASL_STEP,
                  (xdrproc_t) xdr_remote_auth_sasl_step_args, (char *) &pargs,
                  (xdrproc_t) xdr_remote_auth_sasl_step_ret, (char *) &pret) != 0)
diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c
index 1df43bb044..98be120a11 100644
--- a/src/rpc/virnetsshsession.c
+++ b/src/rpc/virnetsshsession.c
@@ -271,7 +271,6 @@ virNetSSHCheckHostKey(virNetSSHSession *sess)
     size_t keyLength;
     char *errmsg;
     g_auto(virBuffer) buff = VIR_BUFFER_INITIALIZER;
-    virConnectCredential askKey;
     struct libssh2_knownhost *knownHostEntry = NULL;
     size_t i;
     char *hostnameStr = NULL;
@@ -303,6 +302,8 @@ virNetSSHCheckHostKey(virNetSSHSession *sess)
     case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND:
         /* key was not found, query to add it to database */
         if (sess->hostKeyVerify == VIR_NET_SSH_HOSTKEY_VERIFY_NORMAL) {
+            virConnectCredential askKey = { 0 };
+
             /* ask to add the key */
             if (!sess->cred || !sess->cred->cb) {
                 virReportError(VIR_ERR_SSH, "%s",
@@ -312,8 +313,6 @@ virNetSSHCheckHostKey(virNetSSHSession *sess)
             }
 
             /* prepare data for the callback */
-            memset(&askKey, 0, sizeof(virConnectCredential));
-
             for (i = 0; i < sess->cred->ncredtype; i++) {
                 if (sess->cred->credtype[i] == VIR_CRED_ECHOPROMPT)
                     break;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index db5f41bae8..cde9d70eef 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -217,12 +217,11 @@ int
 virNetDevMacVLanTapSetup(int *tapfd, size_t tapfdSize, bool vnet_hdr)
 {
     unsigned int features;
-    struct ifreq ifreq;
     short new_flags = 0;
     size_t i;
 
     for (i = 0; i < tapfdSize; i++) {
-        memset(&ifreq, 0, sizeof(ifreq));
+        struct ifreq ifreq = { 0 };
 
         if (ioctl(tapfd[i], TUNGETIFF, &ifreq) < 0) {
             virReportSystemError(errno, "%s",
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index b85b753c96..3e25f18770 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -172,7 +172,6 @@ int virNetDevTapCreate(char **ifname,
                        unsigned int flags)
 {
     size_t i = 0;
-    struct ifreq ifr;
     int rc;
     int ret = -1;
     int fd = -1;
@@ -204,8 +203,9 @@ int virNetDevTapCreate(char **ifname,
     if (!tunpath)
         tunpath = "/dev/net/tun";
 
-    memset(&ifr, 0, sizeof(ifr));
     for (i = 0; i < tapfdSize; i++) {
+        struct ifreq ifr = { 0 };
+
         if ((fd = open(tunpath, O_RDWR)) < 0) {
             virReportSystemError(errno,
                                  _("Unable to open %1$s, is tun module loaded?"),
@@ -213,8 +213,6 @@ int virNetDevTapCreate(char **ifname,
             goto cleanup;
         }
 
-        memset(&ifr, 0, sizeof(ifr));
-
         ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
         /* If tapfdSize is greater than one, request multiqueue */
         if (tapfdSize > 1)
diff --git a/src/util/viruuid.c b/src/util/viruuid.c
index ca22c91d48..a4f3a50fa5 100644
--- a/src/util/viruuid.c
+++ b/src/util/viruuid.c
@@ -215,13 +215,13 @@ int
 virSetHostUUIDStr(const char *uuid)
 {
     int rc;
-    char dmiuuid[VIR_UUID_STRING_BUFLEN];
 
     if (virUUIDIsValid(host_uuid))
         return EEXIST;
 
     if (!uuid) {
-        memset(dmiuuid, 0, sizeof(dmiuuid));
+        char dmiuuid[VIR_UUID_STRING_BUFLEN] = { 0 };
+
         if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) {
             if (!virUUIDParse(dmiuuid, host_uuid))
                 return 0;
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index 2295e29777..0e9cd8a81c 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -44,8 +44,6 @@ static int
 checkProtocols(bool *hasIPv4, bool *hasIPv6,
                int *freePort)
 {
-    struct sockaddr_in in4;
-    struct sockaddr_in6 in6;
     size_t i;
 
     *freePort = 0;
@@ -53,6 +51,8 @@ checkProtocols(bool *hasIPv4, bool *hasIPv6,
         return -1;
 
     for (i = 0; i < 50; i++) {
+        struct sockaddr_in in4 = { 0 };
+        struct sockaddr_in6 in6 = { 0 };
         VIR_AUTOCLOSE s4 = -1;
         VIR_AUTOCLOSE s6 = -1;
 
@@ -71,9 +71,6 @@ checkProtocols(bool *hasIPv4, bool *hasIPv6,
                 return -1;
         }
 
-        memset(&in4, 0, sizeof(in4));
-        memset(&in6, 0, sizeof(in6));
-
         in4.sin_family = AF_INET;
         in4.sin_port = htons(BASE_PORT + i);
         in4.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index ec99b9f99a..c74fc19347 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -444,7 +444,6 @@ cmdDomblkinfoGet(const virDomainBlockInfo *info,
 static bool
 cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
 {
-    virDomainBlockInfo info;
     g_autoptr(virshDomain) dom = NULL;
     bool human = false;
     bool all = false;
@@ -491,6 +490,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
             g_autofree char *cap = NULL;
             g_autofree char *alloc = NULL;
             g_autofree char *phy = NULL;
+            virDomainBlockInfo info = { 0 };
 
             ctxt->node = disks[i];
             protocol = virXPathString("string(./source/@protocol)", ctxt);
@@ -513,10 +513,6 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
                         return false;
                     }
                 }
-            } else {
-                /* if we don't call virDomainGetBlockInfo() who clears 'info'
-                 * we have to do it manually */
-                memset(&info, 0, sizeof(info));
             }
 
             if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human))
@@ -531,6 +527,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
         g_autofree char *cap = NULL;
         g_autofree char *alloc = NULL;
         g_autofree char *phy = NULL;
+        virDomainBlockInfo info = { 0 };
 
         if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
             return false;
-- 
2.41.0
Re: [PATCH 03/13] Decrease scope of some variables
Posted by Claudio Fontana 2 years, 6 months ago
On 8/3/23 12:36, Michal Privoznik wrote:
> There are couple of variables that are declared at function
> beginning but then used solely within a block (either for() loop
> or if() statement). And just before their use they are zeroed
> explicitly using memset(). Decrease their scope, use struct zero
> initializer and drop explicit memset().
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Claudio Fontana <cfontana@suse.de>

> ---
>  src/qemu/qemu_driver.c       | 4 +---
>  src/remote/remote_driver.c   | 7 +++----
>  src/rpc/virnetsshsession.c   | 5 ++---
>  src/util/virnetdevmacvlan.c  | 3 +--
>  src/util/virnetdevtap.c      | 6 ++----
>  src/util/viruuid.c           | 4 ++--
>  tests/virnetsockettest.c     | 7 ++-----
>  tools/virsh-domain-monitor.c | 7 ++-----
>  8 files changed, 15 insertions(+), 28 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 497923ffee..1d7b78d73b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17124,7 +17124,6 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED,
>                              unsigned int privflags G_GNUC_UNUSED)
>  {
>      size_t i;
> -    struct _virDomainInterfaceStats tmp;
>  
>      if (!virDomainObjIsActive(dom))
>          return 0;
> @@ -17135,12 +17134,11 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED,
>      for (i = 0; i < dom->def->nnets; i++) {
>          virDomainNetDef *net = dom->def->nets[i];
>          virDomainNetType actualType;
> +        struct _virDomainInterfaceStats tmp = { 0 };
>  
>          if (!net->ifname)
>              continue;
>  
> -        memset(&tmp, 0, sizeof(tmp));
> -
>          actualType = virDomainNetGetActualType(net);
>  
>          virTypedParamListAddString(params, net->ifname, "net.%zu.name", i);
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 65ec239fb7..d775f65fe2 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -3728,8 +3728,6 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
>      remote_auth_sasl_init_ret iret;
>      remote_auth_sasl_start_args sargs = {0};
>      remote_auth_sasl_start_ret sret;
> -    remote_auth_sasl_step_args pargs = {0};
> -    remote_auth_sasl_step_ret pret;
>      const char *clientout;
>      char *serverin = NULL;
>      size_t clientoutlen, serverinlen;
> @@ -3866,6 +3864,9 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
>       * Even if the server has completed, the client must *always* do at least one step
>       * in this loop to verify the server isn't lying about something. Mutual auth */
>      for (;;) {
> +        remote_auth_sasl_step_args pargs = { 0 };
> +        remote_auth_sasl_step_ret pret = { 0 };
> +
>          if ((err = virNetSASLSessionClientStep(sasl,
>                                                 serverin,
>                                                 serverinlen,
> @@ -3893,14 +3894,12 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
>  
>          /* Not done, prepare to talk with the server for another iteration */
>          /* NB, distinction of NULL vs "" is *critical* in SASL */
> -        memset(&pargs, 0, sizeof(pargs));
>          pargs.nil = clientout ? 0 : 1;
>          pargs.data.data_val = (char*)clientout;
>          pargs.data.data_len = clientoutlen;
>          VIR_DEBUG("Server step with %zu bytes %p",
>                    clientoutlen, clientout);
>  
> -        memset(&pret, 0, sizeof(pret));
>          if (call(conn, priv, 0, REMOTE_PROC_AUTH_SASL_STEP,
>                   (xdrproc_t) xdr_remote_auth_sasl_step_args, (char *) &pargs,
>                   (xdrproc_t) xdr_remote_auth_sasl_step_ret, (char *) &pret) != 0)
> diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c
> index 1df43bb044..98be120a11 100644
> --- a/src/rpc/virnetsshsession.c
> +++ b/src/rpc/virnetsshsession.c
> @@ -271,7 +271,6 @@ virNetSSHCheckHostKey(virNetSSHSession *sess)
>      size_t keyLength;
>      char *errmsg;
>      g_auto(virBuffer) buff = VIR_BUFFER_INITIALIZER;
> -    virConnectCredential askKey;
>      struct libssh2_knownhost *knownHostEntry = NULL;
>      size_t i;
>      char *hostnameStr = NULL;
> @@ -303,6 +302,8 @@ virNetSSHCheckHostKey(virNetSSHSession *sess)
>      case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND:
>          /* key was not found, query to add it to database */
>          if (sess->hostKeyVerify == VIR_NET_SSH_HOSTKEY_VERIFY_NORMAL) {
> +            virConnectCredential askKey = { 0 };
> +
>              /* ask to add the key */
>              if (!sess->cred || !sess->cred->cb) {
>                  virReportError(VIR_ERR_SSH, "%s",
> @@ -312,8 +313,6 @@ virNetSSHCheckHostKey(virNetSSHSession *sess)
>              }
>  
>              /* prepare data for the callback */
> -            memset(&askKey, 0, sizeof(virConnectCredential));
> -
>              for (i = 0; i < sess->cred->ncredtype; i++) {
>                  if (sess->cred->credtype[i] == VIR_CRED_ECHOPROMPT)
>                      break;
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index db5f41bae8..cde9d70eef 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -217,12 +217,11 @@ int
>  virNetDevMacVLanTapSetup(int *tapfd, size_t tapfdSize, bool vnet_hdr)
>  {
>      unsigned int features;
> -    struct ifreq ifreq;
>      short new_flags = 0;
>      size_t i;
>  
>      for (i = 0; i < tapfdSize; i++) {
> -        memset(&ifreq, 0, sizeof(ifreq));
> +        struct ifreq ifreq = { 0 };
>  
>          if (ioctl(tapfd[i], TUNGETIFF, &ifreq) < 0) {
>              virReportSystemError(errno, "%s",
> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> index b85b753c96..3e25f18770 100644
> --- a/src/util/virnetdevtap.c
> +++ b/src/util/virnetdevtap.c
> @@ -172,7 +172,6 @@ int virNetDevTapCreate(char **ifname,
>                         unsigned int flags)
>  {
>      size_t i = 0;
> -    struct ifreq ifr;
>      int rc;
>      int ret = -1;
>      int fd = -1;
> @@ -204,8 +203,9 @@ int virNetDevTapCreate(char **ifname,
>      if (!tunpath)
>          tunpath = "/dev/net/tun";
>  
> -    memset(&ifr, 0, sizeof(ifr));
>      for (i = 0; i < tapfdSize; i++) {
> +        struct ifreq ifr = { 0 };
> +
>          if ((fd = open(tunpath, O_RDWR)) < 0) {
>              virReportSystemError(errno,
>                                   _("Unable to open %1$s, is tun module loaded?"),
> @@ -213,8 +213,6 @@ int virNetDevTapCreate(char **ifname,
>              goto cleanup;
>          }
>  
> -        memset(&ifr, 0, sizeof(ifr));
> -
>          ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
>          /* If tapfdSize is greater than one, request multiqueue */
>          if (tapfdSize > 1)
> diff --git a/src/util/viruuid.c b/src/util/viruuid.c
> index ca22c91d48..a4f3a50fa5 100644
> --- a/src/util/viruuid.c
> +++ b/src/util/viruuid.c
> @@ -215,13 +215,13 @@ int
>  virSetHostUUIDStr(const char *uuid)
>  {
>      int rc;
> -    char dmiuuid[VIR_UUID_STRING_BUFLEN];
>  
>      if (virUUIDIsValid(host_uuid))
>          return EEXIST;
>  
>      if (!uuid) {
> -        memset(dmiuuid, 0, sizeof(dmiuuid));
> +        char dmiuuid[VIR_UUID_STRING_BUFLEN] = { 0 };
> +
>          if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) {
>              if (!virUUIDParse(dmiuuid, host_uuid))
>                  return 0;
> diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
> index 2295e29777..0e9cd8a81c 100644
> --- a/tests/virnetsockettest.c
> +++ b/tests/virnetsockettest.c
> @@ -44,8 +44,6 @@ static int
>  checkProtocols(bool *hasIPv4, bool *hasIPv6,
>                 int *freePort)
>  {
> -    struct sockaddr_in in4;
> -    struct sockaddr_in6 in6;
>      size_t i;
>  
>      *freePort = 0;
> @@ -53,6 +51,8 @@ checkProtocols(bool *hasIPv4, bool *hasIPv6,
>          return -1;
>  
>      for (i = 0; i < 50; i++) {
> +        struct sockaddr_in in4 = { 0 };
> +        struct sockaddr_in6 in6 = { 0 };
>          VIR_AUTOCLOSE s4 = -1;
>          VIR_AUTOCLOSE s6 = -1;
>  
> @@ -71,9 +71,6 @@ checkProtocols(bool *hasIPv4, bool *hasIPv6,
>                  return -1;
>          }
>  
> -        memset(&in4, 0, sizeof(in4));
> -        memset(&in6, 0, sizeof(in6));
> -
>          in4.sin_family = AF_INET;
>          in4.sin_port = htons(BASE_PORT + i);
>          in4.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index ec99b9f99a..c74fc19347 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -444,7 +444,6 @@ cmdDomblkinfoGet(const virDomainBlockInfo *info,
>  static bool
>  cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>  {
> -    virDomainBlockInfo info;
>      g_autoptr(virshDomain) dom = NULL;
>      bool human = false;
>      bool all = false;
> @@ -491,6 +490,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>              g_autofree char *cap = NULL;
>              g_autofree char *alloc = NULL;
>              g_autofree char *phy = NULL;
> +            virDomainBlockInfo info = { 0 };
>  
>              ctxt->node = disks[i];
>              protocol = virXPathString("string(./source/@protocol)", ctxt);
> @@ -513,10 +513,6 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>                          return false;
>                      }
>                  }
> -            } else {
> -                /* if we don't call virDomainGetBlockInfo() who clears 'info'
> -                 * we have to do it manually */
> -                memset(&info, 0, sizeof(info));
>              }
>  
>              if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human))
> @@ -531,6 +527,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>          g_autofree char *cap = NULL;
>          g_autofree char *alloc = NULL;
>          g_autofree char *phy = NULL;
> +        virDomainBlockInfo info = { 0 };
>  
>          if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
>              return false;