bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: relocatable: avoid compiler warnings (-Wshadow)


From: Bruno Haible
Subject: Re: relocatable: avoid compiler warnings (-Wshadow)
Date: Thu, 24 Jan 2019 02:00:12 +0100
User-agent: KMail/5.1.3 (Linux/4.4.0-141-generic; KDE/5.18.0; x86_64; ; )

Hi Akim,

> diff --git a/lib/relocatable.c b/lib/relocatable.c
> index 17cdb6590..033c44139 100644
> --- a/lib/relocatable.c
> +++ b/lib/relocatable.c
> @@ -282,23 +282,21 @@ compute_curr_prefix (const char *orig_installprefix,
>        }
>  
>      {
> -      size_t curr_prefix_len = cp - curr_installdir;
> -      char *curr_prefix;
> -
> -      curr_prefix = (char *) xmalloc (curr_prefix_len + 1);
> +      size_t prefix_len = cp - curr_installdir;
> +      char *prefix = (char *) xmalloc (prefix_len + 1);
>  #ifdef NO_XMALLOC
> -      if (curr_prefix == NULL)
> +      if (prefix == NULL)
>          {
>            free (curr_installdir);
>            return NULL;
>          }
>  #endif
> -      memcpy (curr_prefix, curr_installdir, curr_prefix_len);
> -      curr_prefix[curr_prefix_len] = '\0';
> +      memcpy (prefix, curr_installdir, prefix_len);
> +      prefix[prefix_len] = '\0';
>  
>        free (curr_installdir);
>  
> -      return curr_prefix;
> +      return prefix;
>      }
>    }
>  }

It's good to reduce warnings. But I'm unhappy about the choice of
identifiers you made: There are various different prefixes involved
here (install prefix, original prefix, current prefix, etc.), and
the code is complicated, that naming _any_ of it just 'prefix' is
inappropriate. Better a longer, more descriptive name, than a
shorter one.

I'm pushing this commit:


2019-01-23  Akim Demaille  <address@hidden>
            Bruno Haible  <address@hidden>

        relocatable: avoid compiler warnings (-Wshadow)
        * lib/relocatable.c (compute_curr_prefix): Rename local variables
        to avoid name collisions with global variables.

diff --git a/lib/relocatable.c b/lib/relocatable.c
index 17cdb65..de431e7 100644
--- a/lib/relocatable.c
+++ b/lib/relocatable.c
@@ -268,7 +268,7 @@ compute_curr_prefix (const char *orig_installprefix,
           }
         if (!same)
           break;
-        /* The last pathname component was the same.  opi and cpi now point
+        /* The last pathname component was the same.  rpi and cpi now point
            to the slash before it.  */
         rp = rpi;
         cp = cpi;
@@ -282,23 +282,23 @@ compute_curr_prefix (const char *orig_installprefix,
       }
 
     {
-      size_t curr_prefix_len = cp - curr_installdir;
-      char *curr_prefix;
+      size_t computed_curr_prefix_len = cp - curr_installdir;
+      char *computed_curr_prefix;
 
-      curr_prefix = (char *) xmalloc (curr_prefix_len + 1);
+      computed_curr_prefix = (char *) xmalloc (computed_curr_prefix_len + 1);
 #ifdef NO_XMALLOC
-      if (curr_prefix == NULL)
+      if (computed_curr_prefix == NULL)
         {
           free (curr_installdir);
           return NULL;
         }
 #endif
-      memcpy (curr_prefix, curr_installdir, curr_prefix_len);
-      curr_prefix[curr_prefix_len] = '\0';
+      memcpy (computed_curr_prefix, curr_installdir, computed_curr_prefix_len);
+      computed_curr_prefix[computed_curr_prefix_len] = '\0';
 
       free (curr_installdir);
 
-      return curr_prefix;
+      return computed_curr_prefix;
     }
   }
 }





reply via email to

[Prev in Thread] Current Thread [Next in Thread]