Rule 5.3 has the following headline:
"An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope"

The function parameters renamed in this patch are hiding a variable defined
in an enclosing scope or a function identifier.

The following rename is made:
- s/cfg/config/
to distinguish from the variable 'cfg', which is hidden by the parameter inside
the modified functions.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/common/efi/boot.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 24169b7b50..233639f3bc 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -528,10 +528,10 @@ static char * __init split_string(char *s)
     return NULL;
 }
 
-static char *__init get_value(const struct file *cfg, const char *section,
+static char *__init get_value(const struct file *config, const char *section,
                               const char *item)
 {
-    char *ptr = cfg->str, *end = ptr + cfg->size;
+    char *ptr = config->str, *end = ptr + config->size;
     size_t slen = section ? strlen(section) : 0, ilen = strlen(item);
     bool match = !slen;
 
@@ -821,9 +821,9 @@ static bool __init read_section(const EFI_LOADED_IMAGE *image,
     return true;
 }
 
-static void __init pre_parse(const struct file *cfg)
+static void __init pre_parse(const struct file *config)
 {
-    char *ptr = cfg->str, *end = ptr + cfg->size;
+    char *ptr = config->str, *end = ptr + config->size;
     bool start = true, comment = false;
 
     for ( ; ptr < end; ++ptr )
@@ -844,7 +844,7 @@ static void __init pre_parse(const struct file *cfg)
         else
             start = 0;
     }
-    if ( cfg->size && end[-1] )
+    if ( config->size && end[-1] )
          PrintStr(L"No newline at end of config file,"
                    " last line will be ignored.\r\n");
 }
-- 
2.34.1

On 21.07.2023 17:26, Nicola Vetrini wrote:
> Rule 5.3 has the following headline:
> "An identifier declared in an inner scope shall not hide an
> identifier declared in an outer scope"
> 
> The function parameters renamed in this patch are hiding a variable defined
> in an enclosing scope or a function identifier.

I think this sentence should be limited to the part which actually applies.

> The following rename is made:
> - s/cfg/config/
> to distinguish from the variable 'cfg', which is hidden by the parameter inside
> the modified functions.

Hmm. I have to admit that I don't like this. The two functions in question
always have "&cfg" passed to them. So using the same name is, well, kind of
intentional. If we really need to change the code, I guess we may want to
consider dropping the parameter and always-same arguments (albeit the
choice to have this parameter was also intentional).

Another route would be to truly generalize the parameters by naming them
"file", like other functions have it.

Jan

On 24/07/23 10:07, Jan Beulich wrote:
> On 21.07.2023 17:26, Nicola Vetrini wrote:
>> Rule 5.3 has the following headline:
>> "An identifier declared in an inner scope shall not hide an
>> identifier declared in an outer scope"
>>
>> The function parameters renamed in this patch are hiding a variable defined
>> in an enclosing scope or a function identifier.
> 
> I think this sentence should be limited to the part which actually applies.
> 

Yes, I'll fix this.

>> The following rename is made:
>> - s/cfg/config/
>> to distinguish from the variable 'cfg', which is hidden by the parameter inside
>> the modified functions.
> 
> Hmm. I have to admit that I don't like this. The two functions in question
> always have "&cfg" passed to them. So using the same name is, well, kind of
> intentional. If we really need to change the code, I guess we may want to
> consider dropping the parameter and always-same arguments (albeit the
> choice to have this parameter was also intentional).
> 
> Another route would be to truly generalize the parameters by naming them
> "file", like other functions have it.
> 

Temporarily the second option seems the best, since further refactorings 
can be discussed properly later. I chose 'config' because it seemed to 
carry the same information as in the original source, but I'm open to 
other names such as "file" (though I'll test this before submitting a v2 
to avoid unintentionally adding violations).

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

On Fri, 21 Jul 2023, Nicola Vetrini wrote:
> Rule 5.3 has the following headline:
> "An identifier declared in an inner scope shall not hide an
> identifier declared in an outer scope"
> 
> The function parameters renamed in this patch are hiding a variable defined
> in an enclosing scope or a function identifier.
> 
> The following rename is made:
> - s/cfg/config/
> to distinguish from the variable 'cfg', which is hidden by the parameter inside
> the modified functions.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/common/efi/boot.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 24169b7b50..233639f3bc 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -528,10 +528,10 @@ static char * __init split_string(char *s)
>      return NULL;
>  }
>  
> -static char *__init get_value(const struct file *cfg, const char *section,
> +static char *__init get_value(const struct file *config, const char *section,
>                                const char *item)
>  {
> -    char *ptr = cfg->str, *end = ptr + cfg->size;
> +    char *ptr = config->str, *end = ptr + config->size;
>      size_t slen = section ? strlen(section) : 0, ilen = strlen(item);
>      bool match = !slen;
>  
> @@ -821,9 +821,9 @@ static bool __init read_section(const EFI_LOADED_IMAGE *image,
>      return true;
>  }
>  
> -static void __init pre_parse(const struct file *cfg)
> +static void __init pre_parse(const struct file *config)
>  {
> -    char *ptr = cfg->str, *end = ptr + cfg->size;
> +    char *ptr = config->str, *end = ptr + config->size;
>      bool start = true, comment = false;
>  
>      for ( ; ptr < end; ++ptr )
> @@ -844,7 +844,7 @@ static void __init pre_parse(const struct file *cfg)
>          else
>              start = 0;
>      }
> -    if ( cfg->size && end[-1] )
> +    if ( config->size && end[-1] )
>           PrintStr(L"No newline at end of config file,"
>                     " last line will be ignored.\r\n");
>  }
> -- 
> 2.34.1
>