On Wed, 18 Feb 2015 13:33:21 +0200, Ivan Khoronzhuk wrote:
> There is no reason to pass static vars to function that can use
> only them.
> 
> The dmi_table() can use only dmi_len and dmi_num static vars, so use
> them directly. In this case we can freely change their type in one
> place and slightly decrease redundancy.

Too bad we changed dmi_len's type already ;-)

I can see the benefit, and even if this certainly makes dmi_table
slightly slower, no objection from me.

> Signed-off-by: Ivan Khoronzhuk <[email protected]>
> ---
>  drivers/firmware/dmi_scan.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 3f3470f..8c065f7 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -18,6 +18,8 @@
>  static const char dmi_empty_string[] = "        ";
>  
>  static u32 dmi_ver __initdata;
> +static u32 dmi_len;
> +static u16 dmi_num;

A blank line here wouldn't hurt.

>  /*
>   * Catch too early calls to dmi_check_system():
>   */
> @@ -78,7 +80,7 @@ static const char * __init dmi_string(const struct 
> dmi_header *dm, u8 s)
>   *   We have to be cautious here. We have seen BIOSes with DMI pointers
>   *   pointing to completely the wrong place for example
>   */
> -static void dmi_table(u8 *buf, u32 len, int num,
> +static void dmi_table(u8 *buf,
>                     void (*decode)(const struct dmi_header *, void *),
>                     void *private_data)
>  {
> @@ -89,7 +91,8 @@ static void dmi_table(u8 *buf, u32 len, int num,
>        *      Stop when we see all the items the table claimed to have
>        *      OR we run off the end of the table (also happens)
>        */
> -     while ((i < num) && (data - buf + sizeof(struct dmi_header)) <= len) {
> +     while ((i < dmi_num) && (data - buf + sizeof(struct dmi_header))
> +             <= dmi_len) {

One condition per line is easier to read:

        while ((i < dmi_num) &&
               (data - buf + sizeof(struct dmi_header)) <= dmi_len) {

>               const struct dmi_header *dm = (const struct dmi_header *)data;
>  
>               /*
> @@ -98,9 +101,9 @@ static void dmi_table(u8 *buf, u32 len, int num,
>                *  table in dmi_decode or dmi_string
>                */
>               data += dm->length;
> -             while ((data - buf < len - 1) && (data[0] || data[1]))
> +             while ((data - buf < dmi_len - 1) && (data[0] || data[1]))
>                       data++;
> -             if (data - buf < len - 1)
> +             if (data - buf < dmi_len - 1)
>                       decode(dm, private_data);
>  
>               /*
> @@ -117,8 +120,6 @@ static void dmi_table(u8 *buf, u32 len, int num,
>  static u8 smbios_header[32];
>  static int smbios_header_size;
>  static phys_addr_t dmi_base;
> -static u32 dmi_len;
> -static u16 dmi_num;
>  
>  static int __init dmi_walk_early(void (*decode)(const struct dmi_header *,
>               void *))
> @@ -129,7 +130,7 @@ static int __init dmi_walk_early(void (*decode)(const 
> struct dmi_header *,
>       if (buf == NULL)
>               return -1;
>  
> -     dmi_table(buf, dmi_len, dmi_num, decode, NULL);
> +     dmi_table(buf, decode, NULL);
>  
>       add_device_randomness(buf, dmi_len);
>  
> @@ -913,7 +914,7 @@ int dmi_walk(void (*decode)(const struct dmi_header *, 
> void *),
>       if (buf == NULL)
>               return -1;
>  
> -     dmi_table(buf, dmi_len, dmi_num, decode, private_data);
> +     dmi_table(buf, decode, private_data);
>  
>       dmi_unmap(buf);
>       return 0;

-- 
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to