| Message ID | 1350246738-31699-10-git-send-email-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On 10/15/2012 04:32 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > We don't require the fwts_framework parameter in the following functions: > > fwts_battery_get_count_sys_fs > fwts_battery_get_count_proc_fs > fwts_battery_get_name_sys_fs > fwts_battery_get_name_proc_fs > > ..so remove it. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_battery.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/src/lib/src/fwts_battery.c b/src/lib/src/fwts_battery.c > index 656cc33..b792d0b 100644 > --- a/src/lib/src/fwts_battery.c > +++ b/src/lib/src/fwts_battery.c > @@ -176,7 +176,7 @@ static int fwts_battery_get_capacity_proc_fs(fwts_framework *fw, > return FWTS_OK; > } > > -static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *count) > +static int fwts_battery_get_count_sys_fs(DIR *dir, int *count) > { > struct dirent *entry; > char path[PATH_MAX]; > @@ -197,7 +197,7 @@ static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *coun > return FWTS_OK; > } > > -static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *count) > +static int fwts_battery_get_count_proc_fs(DIR *dir, int *count) > { > struct dirent *entry; > do { > @@ -208,7 +208,7 @@ static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *cou > return FWTS_OK; > } > > -static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, char *name) > +static int fwts_battery_get_name_sys_fs(DIR *dir, int index, char *name) > { > struct dirent *entry; > char path[PATH_MAX]; > @@ -242,7 +242,7 @@ static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, > return FWTS_ERROR; > } > > -static int fwts_battery_get_name_proc_fs(fwts_framework *fw, DIR *dir, int index, char *name) > +static int fwts_battery_get_name_proc_fs(DIR *dir, int index, char *name) > { > struct dirent *entry; > int i = 0; > @@ -591,10 +591,10 @@ int fwts_battery_get_name(fwts_framework *fw, int index, char *name) > DIR *dir; > > if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { > - ret = fwts_battery_get_name_sys_fs(fw, dir, index, name); > + ret = fwts_battery_get_name_sys_fs(dir, index, name); > closedir(dir); > } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { > - ret = fwts_battery_get_name_proc_fs(fw, dir, index, name); > + ret = fwts_battery_get_name_proc_fs(dir, index, name); > closedir(dir); > } else { > return FWTS_ERROR; > @@ -609,10 +609,10 @@ int fwts_battery_get_count(fwts_framework *fw, int *count) > DIR *dir; > > if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { > - ret = fwts_battery_get_count_sys_fs(fw, dir, count); > + ret = fwts_battery_get_count_sys_fs(dir, count); > closedir(dir); > } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { > - ret = fwts_battery_get_count_proc_fs(fw, dir, count); > + ret = fwts_battery_get_count_proc_fs(dir, count); > closedir(dir); > } else { > return FWTS_ERROR; > Functions such as fwts_battery_get_capacity, fwts_battery_get_capacity_proc_fs and fwts_battery_get_capacity_sys_fs still have the fwts_framework parameter and it enables *fwts_log_info* to log. Even though fwts_framework is not used now, will it still be better to keep them for consistency? (Removing them and adding them back when necessary are just as good. It is just to a second opinion.) Cheers, Alex Hung
On 15/10/12 04:34, Alex Hung wrote: > On 10/15/2012 04:32 AM, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> We don't require the fwts_framework parameter in the following functions: >> >> fwts_battery_get_count_sys_fs >> fwts_battery_get_count_proc_fs >> fwts_battery_get_name_sys_fs >> fwts_battery_get_name_proc_fs >> >> ..so remove it. >> >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> src/lib/src/fwts_battery.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/src/lib/src/fwts_battery.c b/src/lib/src/fwts_battery.c >> index 656cc33..b792d0b 100644 >> --- a/src/lib/src/fwts_battery.c >> +++ b/src/lib/src/fwts_battery.c >> @@ -176,7 +176,7 @@ static int >> fwts_battery_get_capacity_proc_fs(fwts_framework *fw, >> return FWTS_OK; >> } >> >> -static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR >> *dir, int *count) >> +static int fwts_battery_get_count_sys_fs(DIR *dir, int *count) >> { >> struct dirent *entry; >> char path[PATH_MAX]; >> @@ -197,7 +197,7 @@ static int >> fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *coun >> return FWTS_OK; >> } >> >> -static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR >> *dir, int *count) >> +static int fwts_battery_get_count_proc_fs(DIR *dir, int *count) >> { >> struct dirent *entry; >> do { >> @@ -208,7 +208,7 @@ static int >> fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *cou >> return FWTS_OK; >> } >> >> -static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, >> int index, char *name) >> +static int fwts_battery_get_name_sys_fs(DIR *dir, int index, char *name) >> { >> struct dirent *entry; >> char path[PATH_MAX]; >> @@ -242,7 +242,7 @@ static int >> fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, >> return FWTS_ERROR; >> } >> >> -static int fwts_battery_get_name_proc_fs(fwts_framework *fw, DIR >> *dir, int index, char *name) >> +static int fwts_battery_get_name_proc_fs(DIR *dir, int index, char >> *name) >> { >> struct dirent *entry; >> int i = 0; >> @@ -591,10 +591,10 @@ int fwts_battery_get_name(fwts_framework *fw, >> int index, char *name) >> DIR *dir; >> >> if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { >> - ret = fwts_battery_get_name_sys_fs(fw, dir, index, name); >> + ret = fwts_battery_get_name_sys_fs(dir, index, name); >> closedir(dir); >> } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { >> - ret = fwts_battery_get_name_proc_fs(fw, dir, index, name); >> + ret = fwts_battery_get_name_proc_fs(dir, index, name); >> closedir(dir); >> } else { >> return FWTS_ERROR; >> @@ -609,10 +609,10 @@ int fwts_battery_get_count(fwts_framework *fw, >> int *count) >> DIR *dir; >> >> if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { >> - ret = fwts_battery_get_count_sys_fs(fw, dir, count); >> + ret = fwts_battery_get_count_sys_fs(dir, count); >> closedir(dir); >> } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { >> - ret = fwts_battery_get_count_proc_fs(fw, dir, count); >> + ret = fwts_battery_get_count_proc_fs(dir, count); >> closedir(dir); >> } else { >> return FWTS_ERROR; >> > > Functions such as fwts_battery_get_capacity, > fwts_battery_get_capacity_proc_fs and fwts_battery_get_capacity_sys_fs > still have the fwts_framework parameter and it enables *fwts_log_info* > to log. Indeed - but the helper functions don't need to log anything, so lets remove redundant code.. > > Even though fwts_framework is not used now, will it still be better to > keep them for consistency? (Removing them and adding them back when > necessary are just as good. It is just to a second opinion.) Good question :-) Since these are internal helper functions and don't change the overall API then I'm keen to remove any old redundant or unnecessary cruft from the code, especially as I'd ultimately like to use the -Wextra flag once fwts is really in shape for this as -Wextra can find buggy code. Colin > > Cheers, > Alex Hung > > >
On 10/15/2012 04:32 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > We don't require the fwts_framework parameter in the following functions: > > fwts_battery_get_count_sys_fs > fwts_battery_get_count_proc_fs > fwts_battery_get_name_sys_fs > fwts_battery_get_name_proc_fs > > ..so remove it. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_battery.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/src/lib/src/fwts_battery.c b/src/lib/src/fwts_battery.c > index 656cc33..b792d0b 100644 > --- a/src/lib/src/fwts_battery.c > +++ b/src/lib/src/fwts_battery.c > @@ -176,7 +176,7 @@ static int fwts_battery_get_capacity_proc_fs(fwts_framework *fw, > return FWTS_OK; > } > > -static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *count) > +static int fwts_battery_get_count_sys_fs(DIR *dir, int *count) > { > struct dirent *entry; > char path[PATH_MAX]; > @@ -197,7 +197,7 @@ static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *coun > return FWTS_OK; > } > > -static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *count) > +static int fwts_battery_get_count_proc_fs(DIR *dir, int *count) > { > struct dirent *entry; > do { > @@ -208,7 +208,7 @@ static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *cou > return FWTS_OK; > } > > -static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, char *name) > +static int fwts_battery_get_name_sys_fs(DIR *dir, int index, char *name) > { > struct dirent *entry; > char path[PATH_MAX]; > @@ -242,7 +242,7 @@ static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, > return FWTS_ERROR; > } > > -static int fwts_battery_get_name_proc_fs(fwts_framework *fw, DIR *dir, int index, char *name) > +static int fwts_battery_get_name_proc_fs(DIR *dir, int index, char *name) > { > struct dirent *entry; > int i = 0; > @@ -591,10 +591,10 @@ int fwts_battery_get_name(fwts_framework *fw, int index, char *name) > DIR *dir; > > if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { > - ret = fwts_battery_get_name_sys_fs(fw, dir, index, name); > + ret = fwts_battery_get_name_sys_fs(dir, index, name); > closedir(dir); > } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { > - ret = fwts_battery_get_name_proc_fs(fw, dir, index, name); > + ret = fwts_battery_get_name_proc_fs(dir, index, name); > closedir(dir); > } else { > return FWTS_ERROR; > @@ -609,10 +609,10 @@ int fwts_battery_get_count(fwts_framework *fw, int *count) > DIR *dir; > > if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { > - ret = fwts_battery_get_count_sys_fs(fw, dir, count); > + ret = fwts_battery_get_count_sys_fs(dir, count); > closedir(dir); > } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { > - ret = fwts_battery_get_count_proc_fs(fw, dir, count); > + ret = fwts_battery_get_count_proc_fs(dir, count); > closedir(dir); > } else { > return FWTS_ERROR; > Acked-by: Alex Hung <alex.hung@canonical.com>
On Mon, Oct 15, 2012 at 4:32 AM, Colin King <colin.king@canonical.com> wrote: > From: Colin Ian King <colin.king@canonical.com> > > We don't require the fwts_framework parameter in the following functions: > > fwts_battery_get_count_sys_fs > fwts_battery_get_count_proc_fs > fwts_battery_get_name_sys_fs > fwts_battery_get_name_proc_fs > > ..so remove it. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_battery.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/src/lib/src/fwts_battery.c b/src/lib/src/fwts_battery.c > index 656cc33..b792d0b 100644 > --- a/src/lib/src/fwts_battery.c > +++ b/src/lib/src/fwts_battery.c > @@ -176,7 +176,7 @@ static int fwts_battery_get_capacity_proc_fs(fwts_framework *fw, > return FWTS_OK; > } > > -static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *count) > +static int fwts_battery_get_count_sys_fs(DIR *dir, int *count) > { > struct dirent *entry; > char path[PATH_MAX]; > @@ -197,7 +197,7 @@ static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *coun > return FWTS_OK; > } > > -static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *count) > +static int fwts_battery_get_count_proc_fs(DIR *dir, int *count) > { > struct dirent *entry; > do { > @@ -208,7 +208,7 @@ static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *cou > return FWTS_OK; > } > > -static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, char *name) > +static int fwts_battery_get_name_sys_fs(DIR *dir, int index, char *name) > { > struct dirent *entry; > char path[PATH_MAX]; > @@ -242,7 +242,7 @@ static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, > return FWTS_ERROR; > } > > -static int fwts_battery_get_name_proc_fs(fwts_framework *fw, DIR *dir, int index, char *name) > +static int fwts_battery_get_name_proc_fs(DIR *dir, int index, char *name) > { > struct dirent *entry; > int i = 0; > @@ -591,10 +591,10 @@ int fwts_battery_get_name(fwts_framework *fw, int index, char *name) > DIR *dir; > > if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { > - ret = fwts_battery_get_name_sys_fs(fw, dir, index, name); > + ret = fwts_battery_get_name_sys_fs(dir, index, name); > closedir(dir); > } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { > - ret = fwts_battery_get_name_proc_fs(fw, dir, index, name); > + ret = fwts_battery_get_name_proc_fs(dir, index, name); > closedir(dir); > } else { > return FWTS_ERROR; > @@ -609,10 +609,10 @@ int fwts_battery_get_count(fwts_framework *fw, int *count) > DIR *dir; > > if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { > - ret = fwts_battery_get_count_sys_fs(fw, dir, count); > + ret = fwts_battery_get_count_sys_fs(dir, count); > closedir(dir); > } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { > - ret = fwts_battery_get_count_proc_fs(fw, dir, count); > + ret = fwts_battery_get_count_proc_fs(dir, count); > closedir(dir); > } else { > return FWTS_ERROR; > -- > 1.7.10.4 > Acked-by: Keng-Yu Lin <kengyu@canonical.com>
diff --git a/src/lib/src/fwts_battery.c b/src/lib/src/fwts_battery.c index 656cc33..b792d0b 100644 --- a/src/lib/src/fwts_battery.c +++ b/src/lib/src/fwts_battery.c @@ -176,7 +176,7 @@ static int fwts_battery_get_capacity_proc_fs(fwts_framework *fw, return FWTS_OK; } -static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *count) +static int fwts_battery_get_count_sys_fs(DIR *dir, int *count) { struct dirent *entry; char path[PATH_MAX]; @@ -197,7 +197,7 @@ static int fwts_battery_get_count_sys_fs(fwts_framework *fw, DIR *dir, int *coun return FWTS_OK; } -static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *count) +static int fwts_battery_get_count_proc_fs(DIR *dir, int *count) { struct dirent *entry; do { @@ -208,7 +208,7 @@ static int fwts_battery_get_count_proc_fs(fwts_framework *fw, DIR *dir, int *cou return FWTS_OK; } -static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, char *name) +static int fwts_battery_get_name_sys_fs(DIR *dir, int index, char *name) { struct dirent *entry; char path[PATH_MAX]; @@ -242,7 +242,7 @@ static int fwts_battery_get_name_sys_fs(fwts_framework *fw, DIR *dir, int index, return FWTS_ERROR; } -static int fwts_battery_get_name_proc_fs(fwts_framework *fw, DIR *dir, int index, char *name) +static int fwts_battery_get_name_proc_fs(DIR *dir, int index, char *name) { struct dirent *entry; int i = 0; @@ -591,10 +591,10 @@ int fwts_battery_get_name(fwts_framework *fw, int index, char *name) DIR *dir; if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { - ret = fwts_battery_get_name_sys_fs(fw, dir, index, name); + ret = fwts_battery_get_name_sys_fs(dir, index, name); closedir(dir); } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { - ret = fwts_battery_get_name_proc_fs(fw, dir, index, name); + ret = fwts_battery_get_name_proc_fs(dir, index, name); closedir(dir); } else { return FWTS_ERROR; @@ -609,10 +609,10 @@ int fwts_battery_get_count(fwts_framework *fw, int *count) DIR *dir; if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) { - ret = fwts_battery_get_count_sys_fs(fw, dir, count); + ret = fwts_battery_get_count_sys_fs(dir, count); closedir(dir); } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) { - ret = fwts_battery_get_count_proc_fs(fw, dir, count); + ret = fwts_battery_get_count_proc_fs(dir, count); closedir(dir); } else { return FWTS_ERROR;