Skip to content

Commit

Permalink
eeepc-laptop: simplify parse_arg()
Browse files Browse the repository at this point in the history
parse_arg() has three possible return values:
    -EINVAL if sscanf(), in short, fails;
    zero if "count" is zero; and
    "count" in all other cases

But "count" will never be zero. See, parse_arg() is called by the
various store functions. And the callchain of these functions starts
with sysfs_kf_write(). And that function checks for a zero "count". So
we can stop checking for a zero "count", drop the "count" argument
entirely, and transform parse_arg() into a function that returns zero on
success or a negative error. That, in turn, allows to make those store
functions just return "count" on success. The net effect is that the
code becomes a bit easier to understand.

A nice side effect is that this GCC warning is silenced too:
    drivers/platform/x86/eeepc-laptop.c: In function ‘store_sys_acpi’:
    drivers/platform/x86/eeepc-laptop.c:279:10: warning: ‘value’ may be used uninitialized in this function [-Wmaybe-uninitialized]
      int rv, value;

Which is, of course, the reason to have a look at parse_arg().

Signed-off-by: Paul Bolle <[email protected]>
Signed-off-by: Darren Hart <[email protected]>
  • Loading branch information
pebolle authored and dvhart committed Sep 18, 2014
1 parent a3d3c53 commit 95369a7
Showing 1 changed file with 18 additions and 18 deletions.
36 changes: 18 additions & 18 deletions drivers/platform/x86/eeepc-laptop.c
Original file line number Diff line number Diff line change
Expand Up @@ -263,13 +263,11 @@ static int acpi_setter_handle(struct eeepc_laptop *eeepc, int cm,
/*
* Sys helpers
*/
static int parse_arg(const char *buf, unsigned long count, int *val)
static int parse_arg(const char *buf, int *val)
{
if (!count)
return 0;
if (sscanf(buf, "%i", val) != 1)
return -EINVAL;
return count;
return 0;
}

static ssize_t store_sys_acpi(struct device *dev, int cm,
Expand All @@ -278,12 +276,13 @@ static ssize_t store_sys_acpi(struct device *dev, int cm,
struct eeepc_laptop *eeepc = dev_get_drvdata(dev);
int rv, value;

rv = parse_arg(buf, count, &value);
if (rv > 0)
value = set_acpi(eeepc, cm, value);
if (value < 0)
rv = parse_arg(buf, &value);
if (rv < 0)
return rv;
rv = set_acpi(eeepc, cm, value);
if (rv < 0)
return -EIO;
return rv;
return count;
}

static ssize_t show_sys_acpi(struct device *dev, int cm, char *buf)
Expand Down Expand Up @@ -377,13 +376,13 @@ static ssize_t store_cpufv(struct device *dev,
return -EPERM;
if (get_cpufv(eeepc, &c))
return -ENODEV;
rv = parse_arg(buf, count, &value);
rv = parse_arg(buf, &value);
if (rv < 0)
return rv;
if (!rv || value < 0 || value >= c.num)
if (value < 0 || value >= c.num)
return -EINVAL;
set_acpi(eeepc, CM_ASL_CPUFV, value);
return rv;
return count;
}

static ssize_t show_cpufv_disabled(struct device *dev,
Expand All @@ -402,7 +401,7 @@ static ssize_t store_cpufv_disabled(struct device *dev,
struct eeepc_laptop *eeepc = dev_get_drvdata(dev);
int rv, value;

rv = parse_arg(buf, count, &value);
rv = parse_arg(buf, &value);
if (rv < 0)
return rv;

Expand All @@ -412,7 +411,7 @@ static ssize_t store_cpufv_disabled(struct device *dev,
pr_warn("cpufv enabled (not officially supported "
"on this model)\n");
eeepc->cpufv_disabled = false;
return rv;
return count;
case 1:
return -EPERM;
default:
Expand Down Expand Up @@ -1042,10 +1041,11 @@ static ssize_t store_sys_hwmon(void (*set)(int), const char *buf, size_t count)
{
int rv, value;

rv = parse_arg(buf, count, &value);
if (rv > 0)
set(value);
return rv;
rv = parse_arg(buf, &value);
if (rv < 0)
return rv;
set(value);
return count;
}

static ssize_t show_sys_hwmon(int (*get)(void), char *buf)
Expand Down

0 comments on commit 95369a7

Please sign in to comment.