Bug#715633: Crash: Proposing fix
The crash referenced in the mayhem data occurs whenever the "--pidfile"
option is used (no matter what the argument).
The problem has been introduced by Debian, in the debian patch
"add_pidfile_option.patch": The new argument "pidfile" is defined with
the "no_argument" specifier, but the argument processing switch tries to
call "strdup" on "optarg" - which is never set.
Trivial fix:
As the "pidfile" option doesn't have any effect, if "gconf.pidfile"
isn't set to anything non-NULL, I guess that the argument is really
mandatory. So change the "no_argument" specifier in line 102 of the
patch to "required_argument".
I attach a new (fixed) version of the patch.
Cheers,
Martin
--- ample-0.5.7.orig/src/ample.c
+++ ample-0.5.7/src/ample.c
@@ -241,6 +241,26 @@
return;
}
+/*
+ * Write a pidfile
+ *
+ * Added by Debian Maintainer
+ *
+ */
+static int
+do_pidfile(void)
+{
+ FILE *f;
+ int pidnum;
+ pidnum = getpid();
+ f = fopen(gconf.pidfile, "w");
+ if (!f) {
+ return (1);
+ }
+ fprintf(f,"%d\n", pidnum);
+ fclose(f);
+ return(0);
+}
/*
* Checks if a file descriptor is a socket.
@@ -292,6 +312,15 @@
die("fork");
}
+/* Remove pidfile when dies
+ *
+ */
+static void
+removepidfile(void)
+{
+ unlink(gconf.pidfile);
+ exit(EXIT_SUCCESS);
+}
/*
* Prepares the proper way for ample to log messages.
@@ -460,6 +489,18 @@
checkopt(argc, argv);
gconf.inetd = issocket(0);
daemonize();
+
+ /* Added by Debian Maintainer */
+ if (gconf.pidfile) {
+ debug (1, "pidfile:%s \n", gconf.pidfile);
+ if (do_pidfile() != 0) {
+ debug(1, "could not write pidfile");
+ exit(1);
+ }
+
+ }
+ /**/
+
preparelog();
if(!gconf.inetd)
logmsg("Ample/%s started\n", AMPLE_VERSION);
@@ -488,6 +529,12 @@
memset(&sa, 0, sizeof(sa));
sa.sa_handler = sigchild_handler;
sigaction(SIGCHLD, &sa, NULL);
+
+ /* Added by the Debian Maintainer */
+ if (signal(SIGTERM, removepidfile) == SIG_ERR) {
+ debug(1, "err, I can't quit");
+ }
+ /**/
sigemptyset(&chld);
sigaddset(&chld, SIGCHLD);
--- ample-0.5.7.orig/src/ample.h
+++ ample-0.5.7/src/ample.h
@@ -36,6 +36,7 @@
char * servername;
char * serveraddress;
char * filter;
+ char * pidfile;
};
struct childstat {
--- ample-0.5.7.orig/src/configuration.c
+++ ample-0.5.7/src/configuration.c
@@ -621,6 +621,7 @@
{"servername", OPT_STRING, &gconf.servername},
{"serveraddress", OPT_STRING, &gconf.serveraddress},
{"username", OPT_STRING, &gconf.username},
+ {"pidfile", OPT_STRING, &gconf.pidfile},
{NULL, 0, NULL}
};
@@ -674,13 +675,14 @@
{"debug", optional_argument, NULL, 'd'},
{"trace", no_argument, NULL, 't'},
{"version", no_argument, NULL, 'v'},
+ {"pidfile", required_argument, NULL, 'i'},
{NULL, 0, NULL, 0}
};
- while((c = getopt_long(argc, argv, "p:oc:nf:m:hd::tv", longopts, &i))
+ while((c = getopt_long(argc, argv, "p:oc:nf:m:hd::ti:v", longopts, &i))
!= -1) {
#else
- while((c = getopt(argc, argv, "p:oc:nf:m:hd::tv")) != -1) {
+ while((c = getopt(argc, argv, "p:oc:nf:m:hd::ti:v")) != -1) {
#endif
switch(c) {
case 'p':
@@ -718,6 +720,9 @@
case 'v':
printf("Ample version %s\n", AMPLE_VERSION);
exit(0);
+ case 'i':
+ gconf.pidfile = strdup(optarg);
+ break;
default:
usage(TRUE);
}
--- ample-0.5.7.orig/src/configuration.h
+++ ample-0.5.7/src/configuration.h
@@ -70,6 +70,7 @@
higher number means more detail\n\
-t, --trace no forking, no backgrounding\n\
helpful when debugging\n\
+ -i, --pidfile create a pidfile \n\
-v, --version output version information and exit\n\
\n\
Report bugs to <david@2gen.com>\n"
@@ -86,7 +87,8 @@
-d[=NUMBER] debug messages will be printed,\n\
higher number means more detail\n\
-t no forking, no backgrounding,\n\
- helpful when debugging)\n\
+ (helpful when debugging)\n\
+ -i create a pidfile \n\
-v output version information and exit\n\
\n\
Report bugs to <david@2gen.com>\n"
--- ample-0.5.7.orig/docs/ample.1.in
+++ ample-0.5.7/docs/ample.1.in
@@ -86,6 +86,9 @@
.B -v --version
Display version information and exit.
.TP
+.B -i --pidfile [PIDFILE]
+Write a PIDFILE to handle ample as a daemon
+.TP
.BI "[" "PATH" "...]"
These are path(s) to files or directories that Ample can use to
populate it's list of MP3's. If
Reply to: