lkml.org 
[lkml]   [2015]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [Patch v2 3/3] firmware: dmi_scan: use direct access to static vars
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 <ivan.khoronzhuk@linaro.org>
> ---
> 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


\
 
 \ /
  Last update: 2015-04-13 20:01    [W:0.149 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site