Bug#771844: (pre-approval) unblock: hdapsd/1:20141024-3 or 1:201412xx-1
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 */
Reply to: