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

Bug#771844: marked as done ((pre-approval) unblock: hdapsd/1:20141024-3 or 1:201412xx-1)



Your message dated Wed, 3 Dec 2014 15:45:36 +0100
with message-id <20141203144536.GB2338@ugent.be>
and subject line Re: Bug#771844: (pre-approval) unblock: hdapsd/1:20141024-3 or 1:201412xx-1
has caused the Debian Bug report #771844,
regarding (pre-approval) unblock: hdapsd/1:20141024-3 or 1:201412xx-1
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.)


-- 
771844: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=771844
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 RT,

sorry to bother you again, but there is an important bug in hdapsd I'd
like to fix for jessie:
#770069 [i] [hdapsd] fails to stop without SIGKILL on FREEFALL hardware

There is a patch [1], which is currently tested by the reporter (I do not
own the needed hardware). As soon I get positive results, I'd like to
release a fix upstream and upload it to Debian.

As I will be releasing a new upstream anyways, my prefered fix for #770069
would be that very release in Debian. This would result in the following
(filtered) debdiff in hdapsd-20141202-1.debdiff:
 configure.ac                     |    2 +-
 hdapsd-20141202/debian/rules     |    4 ----
 hdapsd-20141202/misc/Makefile.am |    1 +
 hdapsd-20141202/src/hdapsd.c     |   24 +++++++++++++++++-------
 4 files changed, 19 insertions(+), 12 deletions(-)

configure is the version bump, hdapsd.c is the fix.
misc/Makefile is a "make clean" fix for systemd units, which previously was
in d/rules (that's the 4 deletes in there). the files were not properly
removed before.

If you don't like the new upstream version, I could also backport the fix to
the package in jessie, which would result in the following debdiff:
hdapsd-20141024-3.debdiff  
 series                                |    1 
 use-sigaction-instead-of-signal.patch |   77 ++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
(it's the same patch to hdapsd.c as above, plus the usual git headers).

Please tell me which way you prefer.

Thanks in advance
Evgeni

[1] https://github.com/evgeni/hdapsd/commit/746d64e95021d6e3ab369687373f3ed58c2799b7

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

Kernel: Linux 3.16.0-4-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
diff --git a/debian/patches/series b/debian/patches/series
new file mode 100644
index 0000000..8751803
--- /dev/null
+++ b/debian/patches/series
@@ -0,0 +1 @@
+use-sigaction-instead-of-signal.patch
diff --git a/debian/patches/use-sigaction-instead-of-signal.patch b/debian/patches/use-sigaction-instead-of-signal.patch
new file mode 100644
index 0000000..20a115e
--- /dev/null
+++ b/debian/patches/use-sigaction-instead-of-signal.patch
@@ -0,0 +1,77 @@
+From 746d64e95021d6e3ab369687373f3ed58c2799b7 Mon Sep 17 00:00:00 2001
+From: Whoopie <whoopie79@gmx.net>
+Date: Wed, 26 Nov 2014 08:06:07 +0100
+Subject: [PATCH] use sigaction(2) instead of signal(2) for handling
+ SIGUSR1/SIGTERM
+
+Debian-Bug: https://bugs.debian.org/770069
+---
+ src/hdapsd.c | 22 ++++++++++++++++------
+ 1 file changed, 16 insertions(+), 6 deletions(-)
+
+diff --git a/src/hdapsd.c b/src/hdapsd.c
+index 5d79dbb..087ab85 100644
+--- a/src/hdapsd.c
++++ b/src/hdapsd.c
+@@ -57,14 +57,14 @@
+ # endif
+ #endif
+ 
++static volatile int pause_now = 0;
++static volatile int running = 1;
+ static int verbose = 0;
+-static int pause_now = 0;
+ static int dry_run = 0;
+ static int poll_sysfs = 0;
+ static int hardware_logic = 0;
+ static int force_software_logic = 0;
+ static int sampling_rate = 0;
+-static int running = 1;
+ static int background = 0;
+ static int dosyslog = 0;
+ static int forcerotational = 0;
+@@ -367,8 +367,9 @@ double get_utime (void)
+  */
+ void SIGUSR1_handler (int sig)
+ {
+-	signal(SIGUSR1, SIGUSR1_handler);
+ 	pause_now = 1;
++	if (verbose)
++		printlog(stdout, "SIGUSR1 called, pause_now is %d", pause_now);
+ }
+ 
+ /*
+@@ -376,8 +377,9 @@ void SIGUSR1_handler (int sig)
+  */
+ void SIGTERM_handler (int sig)
+ {
+-	signal(SIGTERM, SIGTERM_handler);
+ 	running = 0;
++	if (verbose)
++		printlog(stdout, "SIGTERM called, running is %d", running);
+ }
+ 
+ /*
+@@ -1206,9 +1208,17 @@ int main (int argc, char** argv)
+ 	if (verbose)
+ 		printf("sampling_rate: %d\n", sampling_rate);
+ 
+-	signal(SIGUSR1, SIGUSR1_handler);
++	struct sigaction sa;
++	sigemptyset (&sa.sa_mask);
++	sa.sa_flags = 0;
++
++	/* Register the handler for SIGUSR1. */
++	sa.sa_handler = SIGUSR1_handler;
++	sigaction (SIGUSR1, &sa, NULL);
+ 
+-	signal(SIGTERM, SIGTERM_handler);
++	/* Register the handler for SIGTERM. */
++	sa.sa_handler = SIGTERM_handler;
++	sigaction (SIGTERM, &sa, NULL);
+ 
+ 	while (running) {
+ 		if (!hardware_logic) { /* The decision is made by the software */
+-- 
+2.1.3
+
diff -Nru hdapsd-20141024/aclocal.m4 hdapsd-20141202/aclocal.m4
diff -Nru hdapsd-20141024/configure hdapsd-20141202/configure
diff -Nru hdapsd-20141024/configure.ac hdapsd-20141202/configure.ac
--- hdapsd-20141024/configure.ac	2014-10-24 08:43:46.000000000 +0200
+++ hdapsd-20141202/configure.ac	2014-12-02 19:11:42.000000000 +0100
@@ -2,7 +2,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ(2.61)
-AC_INIT(hdapsd, 20141024, hdaps-devel@lists.sourceforge.net)
+AC_INIT(hdapsd, 20141202, hdaps-devel@lists.sourceforge.net)
 AM_INIT_AUTOMAKE([foreign])
 AC_CONFIG_SRCDIR([src/hdapsd.c])
 AC_CONFIG_HEADERS([src/config.h])
diff -Nru hdapsd-20141024/debian/changelog hdapsd-20141202/debian/changelog
diff -Nru hdapsd-20141024/debian/rules hdapsd-20141202/debian/rules
--- hdapsd-20141024/debian/rules	2014-11-16 18:04:47.000000000 +0100
+++ hdapsd-20141202/debian/rules	2014-12-02 19:16:13.000000000 +0100
@@ -10,7 +10,3 @@
 	install -m0644 $(CURDIR)/misc/hdapsd.service $(CURDIR)/debian/hdapsd/lib/systemd/system/
 	mv $(CURDIR)/debian/hdapsd/lib/udev/rules.d/hdapsd.rules $(CURDIR)/debian/hdapsd/usr/share/doc/hdapsd/hdapsd.rules
 	rm -rf $(CURDIR)/debian/hdapsd/lib/udev/
-
-override_dh_auto_clean:
-	dh_auto_clean
-	rm -f $(CURDIR)/misc/*.service
diff -Nru hdapsd-20141024/misc/Makefile.am hdapsd-20141202/misc/Makefile.am
--- hdapsd-20141024/misc/Makefile.am	2014-05-06 18:38:04.000000000 +0200
+++ hdapsd-20141202/misc/Makefile.am	2014-11-18 09:31:45.000000000 +0100
@@ -4,6 +4,7 @@
 systemdsystemunit_DATA = \
 	hdapsd@.service
 noinst_DATA = hdapsd.service
+CLEANFILES = hdapsd@.service hdapsd.service
 
 edit = sed \
 	-e 's|@sbindir[@]|$(sbindir)|g'
diff -Nru hdapsd-20141024/misc/Makefile.in hdapsd-20141202/misc/Makefile.in
diff -Nru hdapsd-20141024/src/hdapsd.c hdapsd-20141202/src/hdapsd.c
--- hdapsd-20141024/src/hdapsd.c	2014-10-24 08:53:29.000000000 +0200
+++ hdapsd-20141202/src/hdapsd.c	2014-11-24 08:07:25.000000000 +0100
@@ -57,14 +57,14 @@
 # endif
 #endif
 
+static volatile int pause_now = 0;
+static volatile int running = 1;
 static int verbose = 0;
-static int pause_now = 0;
 static int dry_run = 0;
 static int poll_sysfs = 0;
 static int hardware_logic = 0;
 static int force_software_logic = 0;
 static int sampling_rate = 0;
-static int running = 1;
 static int background = 0;
 static int dosyslog = 0;
 static int forcerotational = 0;
@@ -367,8 +367,9 @@
  */
 void SIGUSR1_handler (int sig)
 {
-	signal(SIGUSR1, SIGUSR1_handler);
 	pause_now = 1;
+	if (verbose)
+		printlog(stdout, "SIGUSR1 called, pause_now is %d", pause_now);
 }
 
 /*
@@ -376,8 +377,9 @@
  */
 void SIGTERM_handler (int sig)
 {
-	signal(SIGTERM, SIGTERM_handler);
 	running = 0;
+	if (verbose)
+		printlog(stdout, "SIGTERM called, running is %d", running);
 }
 
 /*
@@ -1206,9 +1208,17 @@
 	if (verbose)
 		printf("sampling_rate: %d\n", sampling_rate);
 
-	signal(SIGUSR1, SIGUSR1_handler);
-
-	signal(SIGTERM, SIGTERM_handler);
+	struct sigaction sa;
+	sigemptyset (&sa.sa_mask);
+	sa.sa_flags = 0;
+
+	/* Register the handler for SIGUSR1. */
+	sa.sa_handler = SIGUSR1_handler;
+	sigaction (SIGUSR1, &sa, NULL);
+
+	/* Register the handler for SIGTERM. */
+	sa.sa_handler = SIGTERM_handler;
+	sigaction (SIGTERM, &sa, NULL);
 
 	while (running) {
 		if (!hardware_logic) { /* The decision is made by the software */

--- End Message ---
--- Begin Message ---
Control: tags -1 - moreinfo

Hi,

On Wed, Dec 03, 2014 at 03:09:55PM +0100, Evgeni Golov wrote:
> > Control: tags -1 - moreinfo

This only works if there is no preceding '> '.

> On 12/02/2014 10:36 PM, Adam D. Barratt wrote:
> 
> >> There is a patch [1], which is currently tested by the reporter (I do not
> >> own the needed hardware). As soon I get positive results, I'd like to
> >> release a fix upstream and upload it to Debian.
> > 
> > The new upstream looks fine, thanks. Please go ahead with the upload -
> > bearing in mind that we're getting close to the announced deadline for
> > important-severity fixes - and remove the "moreinfo" tag once the
> > package has been accepted.
> 
> Patch confirmed, upstream released and uploaded to Debian.

Unblocked.

Cheers,

Ivo

--- End Message ---

Reply to: