[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#684588: marked as done (unblock: thinkfan/0.8.1-1 (pre-approval))



Your message dated Mon, 13 Aug 2012 21:48:03 +0100
with message-id <1344890883.1515.9.camel@jacala.jungle.funky-badger.org>
and subject line Re: Bug#684588: unblock: thinkfan/0.8.1-1 (pre-approval)
has caused the Debian Bug report #684588,
regarding unblock: thinkfan/0.8.1-1 (pre-approval)
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
684588: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=684588
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Dear release-team,

I would like to upload thinkfan/0.8.1-1 to unstable and get it into
Wheezy with your help :)
0.8.1 is a bugfix release (diff is attached) which mainly fixes a RC
bug: #684315 Tries to write "level 0" to sysfs, not just "0". The only
hunk that is not for that bug is the following:

--- a/parser.c
+++ b/parser.c
@@ -311,7 +311,10 @@ char *parse_quotation(char **input, const char *mark) {
 	int oldlc = line_count;
 
 	start = *input;
-	if (!char_alt(input, mark, 0)) return NULL;
+	if (!char_alt(input, mark, 0)) {
+		*input = start;
+		return NULL;
+	}
 	ret = char_cat(input, mark, 1);
 	if (!ret) {
 		ret = malloc(sizeof(char));
@@ -321,6 +324,7 @@ char *parse_quotation(char **input, const char *mark) {
 		free(ret);
 		ret = NULL;
 		line_count = oldlc;
+		*input = start;
 	}
 	return ret;
 }

Quoting upstream on it:
When parsing a quotation fails, the **input pointer was not reset to the
beginning of the quotation. This only causes trouble if the quotation
parser successfully reads one or more chars (i.e. something that starts
with "), but then the quotation is not closed with another ". In this
case, an alternative parser would not start where the (failed) quotation
parser started, but where it stopped. However I can't think of any way
this would have caused trouble beyond an inaccurate error message if you
have broken quotations in your config. So nothing to worry about I guess ;-)

→ It's pretty safe not to include it, but it's a bug :)

Would you be okay with uploading 0.8.1-1 to unstable and unblocking it?
Or should I prepare a 0.8.0-2 instead, with the patch w/o parser.c?

Thanks for your work
Evgeni

unblock thinkfan/0.8.1-1

-- System Information:
Debian Release: wheezy/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.4-trunk-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.utf8, LC_CTYPE=en_US.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
diff --git a/config.c b/config.c
index 9ccf794..2b34532 100644
--- a/config.c
+++ b/config.c
@@ -183,7 +183,7 @@ struct tf_config *readconfig(char* fname) {
 		cfg_local->uninit_fan = uninit_fan_ibm;
 	}
 
-	cur_lvl = cfg_local->limits[cfg_local->num_limits - 1].level;
+	lvl_idx = cfg_local->num_limits - 1;
 
 	// configure sensor interface
 	if (cfg_local->num_sensors > 0 &&
@@ -310,6 +310,8 @@ static int add_limit(struct tf_config *cfg, struct limit *limit) {
 	long int tmp;
 	char *end, *conv_lvl;
 
+	limit->sysfslevel = NULL;
+
 	// Check formatting of level string...
 	tmp = strtol(limit->level, &end, 0);
 	if (tmp < INT_MIN || tmp > INT_MAX) {
@@ -324,13 +326,13 @@ static int add_limit(struct tf_config *cfg, struct limit *limit) {
 	}
 	else if (*end == 0) {
 		// just a number
+		limit->sysfslevel = limit->level;
 		conv_lvl = calloc(7 + strlen(limit->level), sizeof(char));
 		snprintf(conv_lvl, 7 + strlen(limit->level), "level %d", (int)tmp);
-		free(limit->level);
 		limit->level = conv_lvl;
 		limit->nlevel = (int)tmp;
 	}
-	else if (sscanf(limit->level, "level %d", (int * )&tmp)) {
+	else if (sscanf(limit->level, "level %d", (int *)&tmp)) {
 		limit->nlevel = (int)tmp;
 	}
 	else if (!strcmp(limit->level, "level disengaged")
@@ -403,6 +405,7 @@ void free_config(struct tf_config *cfg) {
 	}
 	for (j=0; j < cfg->num_limits; j++) {
 		free(cfg->limits[j].level);
+		free(cfg->limits[j].sysfslevel);
 		free(cfg->limits[j].low);
 		free(cfg->limits[j].high);
 	}
diff --git a/globaldefs.h b/globaldefs.h
index c3b9fc2..99099bc 100644
--- a/globaldefs.h
+++ b/globaldefs.h
@@ -65,7 +65,8 @@
 #define unlikely(x)     __builtin_expect((x),0)
 
 struct limit {
-	char *level; // this is written to the fan control file.
+	char *level; // "level x" representation for /proc/acpi/ibm/fan.
+	char *sysfslevel; // numeric representation for /sys/class/hwmon
 	int nlevel;   // A numeric interpretation of the level
 	int *low;   // int array specifying the LOWER limit, terminated by INT_MIN
 	int *high;  // dito for UPPER limit.
@@ -98,7 +99,6 @@ unsigned long int errcnt;
 int *temps, tmax, last_tmax, lvl_idx, *b_tmax, line_count;
 unsigned int chk_sanity, watchdog_timeout, num_temps;
 char *config_file, *prefix, *rbuf,
-	*cur_lvl,
 	errmsg[1024],
 	quiet, nodaemon, resume_is_safe,
 	*oldpwm; // old contents of pwm*_enable, used for uninit_fan()
diff --git a/parser.c b/parser.c
index ab0c0c1..fff923b 100644
--- a/parser.c
+++ b/parser.c
@@ -311,7 +311,10 @@ char *parse_quotation(char **input, const char *mark) {
 	int oldlc = line_count;
 
 	start = *input;
-	if (!char_alt(input, mark, 0)) return NULL;
+	if (!char_alt(input, mark, 0)) {
+		*input = start;
+		return NULL;
+	}
 	ret = char_cat(input, mark, 1);
 	if (!ret) {
 		ret = malloc(sizeof(char));
@@ -321,6 +324,7 @@ char *parse_quotation(char **input, const char *mark) {
 		free(ret);
 		ret = NULL;
 		line_count = oldlc;
+		*input = start;
 	}
 	return ret;
 }
diff --git a/system.c b/system.c
index 7dfb3de..df4999d 100644
--- a/system.c
+++ b/system.c
@@ -109,6 +109,7 @@ int get_temps_ibm() {
  * Set fan speed (IBM interface).
  ***********************************************************/
 void setfan_ibm() {
+	char *cur_lvl = config->limits[lvl_idx].level;
 	int ibm_fan, l = strlen(cur_lvl);
 
 	if (unlikely((ibm_fan = open(IBM_FAN, O_RDWR, O_TRUNC)) < 0)) {
@@ -249,6 +250,7 @@ int get_temps_sysfs() {
  * Set fan speed (sysfs interface).
  ***********************************************************/
 void setfan_sysfs() {
+	char *cur_lvl = config->limits[lvl_idx].sysfslevel;
 	int fan, l = strlen(cur_lvl);
 
 	if (unlikely((fan = open(config->fan, O_WRONLY)) < 0)) {
@@ -322,7 +324,6 @@ void init_fan_sysfs() {
 
 	if ((fd = open(fan_enable, O_WRONLY)) < 0) {
 		report(LOG_ERR, LOG_ERR, "%s: %s\n", fan_enable, strerror(errno));
-		free(fan_enable);
 		errcnt |= ERR_FAN_INIT;
 		goto fail;
 	}
diff --git a/thinkfan.c b/thinkfan.c
index 4f627f1..31291f0 100644
--- a/thinkfan.c
+++ b/thinkfan.c
@@ -36,7 +36,7 @@
 volatile int interrupted;
 unsigned int sleeptime, tmp_sleeptime;
 
-#define set_fan cur_lvl = config->limits[lvl_idx].level; \
+#define set_fan \
 	if (!quiet && nodaemon) \
 	report(LOG_DEBUG, LOG_DEBUG, MSG_DBG_T_STAT); \
 	config->setfan();
@@ -184,7 +184,6 @@ int main(int argc, char **argv) {
 	last_tmax = 0;
 	tmax = 0;
 	temps = NULL;
-	cur_lvl = NULL;
 
 	openlog("thinkfan", LOG_CONS, LOG_USER);
 	syslog(LOG_INFO, "thinkfan " VERSION " starting...");

--- End Message ---
--- Begin Message ---
On Mon, 2012-08-13 at 08:03 +0200, Evgeni Golov wrote:
> On Sun, Aug 12, 2012 at 10:11:22PM +0100, Adam D. Barratt wrote:
> > On Sat, 2012-08-11 at 16:26 +0200, Evgeni Golov wrote:
> > > I would like to upload thinkfan/0.8.1-1 to unstable and get it into
> > > Wheezy with your help :)
> > > 0.8.1 is a bugfix release (diff is attached) which mainly fixes a RC
> > > bug: #684315 Tries to write "level 0" to sysfs, not just "0". The 
> only
> > > hunk that is not for that bug is the following:
> > [...]
> > > → It's pretty safe not to include it, but it's a bug :)
> > >
> > > Would you be okay with uploading 0.8.1-1 to unstable and unblocking 
> it?
> > > Or should I prepare a 0.8.0-2 instead, with the patch w/o parser.c?
> >
> > If the upload happens fairly quickly, I'd be happy with including both
> > sets of changes; thanks.
> 
> Thanks for the quick (p)review. thinkfan hit unstable in the meantime:

Unblocked.

Regards,

Adam

--- End Message ---

Reply to: