Andre Noll [Mon, 16 Apr 2018 17:43:09 +0000 (19:43 +0200)]
aft: Reorder global static variables of aft.c.
These variables constitute the state of the audio file table. The
overall logic is easier to understand if they are combined at one
spot at the top of the file.
The definition of struct ls_data needs to be moved up because of
this. Pure code movement, no semantic changes.
Andre Noll [Wed, 7 Mar 2018 18:45:09 +0000 (19:45 +0100)]
Move MAX_HOSTLEN from server.h to net.h.
This generic networking-related constant does not belong to
server.h. However, since server.h uses the constant, we have to make
sure that net.h is included before server.h.
Andre Noll [Thu, 22 Mar 2018 01:09:43 +0000 (02:09 +0100)]
net: Improve error diagnostics of makesock_addrinfo().
Since the function iterates over all addresses in the passed
addressinfo structure, errors can be non-fatal and may even be
expected. Therefore the function does not log any errors from
socket(2), setsockopt(2), connect(2) or bind(2), but only returns
a generic -E_MAKESOCK error code if none of the addressinfo members
worked. Unfortunately, this means it's impossible to tell from the
log message which of these system calls has failed.
This patch changes the function to also log the errors from the above
system calls, but only with loglevel notice. This way the non-fatal
errors are not shown by default (since the default loglevel is
"warning"), but one can easily activate them by specifying a lower
loglevel.
Andre Noll [Sat, 17 Mar 2018 01:10:55 +0000 (02:10 +0100)]
send_common: Improve error diagnostics of generic_com_on().
Currently the function returns an error code but does not log a message
on errors. The callers, http_send_init() and dccp_send_init(), print
the strerror text with no indication that it is was generic_com_on()
which caused the error.
This commit moves the log messages from the callers to generic_com_on()
and changes the return type of generic_com_on() to void because both
callers now ignore the return value. The new log messages include a
text which tells the user what went wrong.
Andre Noll [Mon, 26 Mar 2018 22:39:48 +0000 (00:39 +0200)]
fd: Let readv_nonblock() recover from EINTR.
No need to fail the operation if the read was interrupted by a signal.
The patch also fixes some minor issues in the documentation:
a typo ("The" was incorrectly capitalized) and a missing \ref
for xwrite(). We use the opportunity to get rid of the \a and \p
font annotations. This improves the readability of the source code,
which is more imporatant than nice looking web pages. However, we
only touch those parts of the documentation which are modified anyway.
Andre Noll [Fri, 5 Jan 2018 15:11:36 +0000 (16:11 +0100)]
mixer: Improve sleep subcommand.
This avoids a pointless exec of "para_client stop" in case no fade-out
mood is given, and another pointless "para_client play" exec which
was executed when already playing.
Andre Noll [Wed, 21 Mar 2018 12:56:04 +0000 (13:56 +0100)]
http_recv: Improve error diagnostics.
If the http receiver is started when the stream is stopped or paused,
para_recv exits silently with no error message. Let's tell the user
what is going on.
Andre Noll [Tue, 9 Jan 2018 23:19:13 +0000 (00:19 +0100)]
afh_recv: Improve error diagnostics.
There are many reasons for afh_recv_open() to fail. For example, the
afh receiver could be unable to open its input file, or the given
begin chunk was larger than the end chunk. At the moment the error
reporting is a bit scarce since only the string of the error code is
printed. This commit makes afh_recv_open() print also the reason for
the error and the name of the file that caused the error.
Andre Noll [Sat, 17 Mar 2018 22:29:12 +0000 (23:29 +0100)]
client: Fix loglevel setting.
If --loglevel is given in the config file but not at the command line,
the value from the config file should be used. However, currently we
use the default value in this case. Moreover, client_parse_config()
sets the loglevel *after* it already printed some log messages.
This commit changes client_parse_config() to initialize the loglevel
pointer right after the command line and config file options have
been merged.
Andre Noll [Thu, 8 Mar 2018 20:39:24 +0000 (21:39 +0100)]
send_common: Remove outdated comment.
Commit 8dab386f (Rework para_accept()) from eight years ago changed
the function to receive also a pointer to the read fd set and pass
this pointer to para_accept() which performs the check for readability
of the socket file descriptor. Since then the comment which says that
the caller must check the fd set is stale.
Andre Noll [Mon, 29 Jan 2018 22:21:11 +0000 (23:21 +0100)]
server: Avoid use of uninitialized memory.
change_current_mood() receives an errmsg pointer which the callers
expect to be initialized with an error string if (and only if) the
function returns negative.
However, most error paths miss to initialize the pointer which results
in undefined behaviour in the caller which attempts to free(3)
uninitialized memory. The gcc AddressSanitizer and valgrind both
catch this:
gcc:
==14788==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x081af250 in thread T0
Andre Noll [Sun, 31 Dec 2017 20:56:35 +0000 (21:56 +0100)]
daemon: Fix log reload for relative paths.
If the argument to --logfile is a relative path, it is interpreted
as relative to the current working directory. In daemon mode, the
current working directory is changed to / during startup. Hence,
when para_server re-opens the log file after it received SIGHUP, the
logfile path will now be interpreted as relative to the the root of
the file system.
Fix this by remembering the original current working directory.
Opening "." as recommended in getcwd(3) is not an option here since the
whole point of changing the cwd to / is to prevent the daemon from
keeping the cwd busy.
Andre Noll [Sun, 31 Dec 2017 19:32:52 +0000 (20:32 +0100)]
Improve daemon_open_log_or_die().
If the log file can not be re-opened, the error message is lost
because the log file has already been closed when PARA_EMERG_LOG() is
called. We can do better by deferring the call to daemon_close_log()
until the new log file has been opened.
With the patch applied, the reason why the (new) log file could not
be opened is logged to the old file.
Andre Noll [Sun, 31 Dec 2017 19:27:57 +0000 (20:27 +0100)]
server: Close logfile later on SIGHUP.
The call to daemon_open_log_or_die() will close the log file just
before it is reopened. This is better because that way any errors
which occur in between won't get lost.
Andre Noll [Sun, 31 Dec 2017 15:27:25 +0000 (16:27 +0100)]
server: Simplify afs socket cookie code.
Currently, the afs_socket_cookie value is passed to afs_init() and
is stored in the commmand task structure of the afs process. This
is unnecessary since the variable defined in server.c is non-static,
and declared as extern in afs.c. We may thus refer to this variable
from afs.c.
The variable was also documented twice. Get rid of one comment.
Andre Noll [Tue, 30 Jan 2018 18:56:16 +0000 (19:56 +0100)]
mixer: Remove default moods for sleep.
The documented behavior of the sleep subcommand is to not fade out
if no fade out mood was given. The current code, however, tries to
switch to the default m/fade. This fails if this mood does not exist,
causing the sleep subcommand to terminate. By removing the default
value we get the documented behaviour.
The --fo-mood and --sleep-mood suffer from the same issue and can be
fixed in the same way.
Andre Noll [Thu, 11 Jan 2018 08:05:13 +0000 (09:05 +0100)]
build: Error out early if flex or bison are not found.
The configure script tests for these executables, but we happily
proceed even if they are not found. For the m4 executable we got
the check right, however. Introduce a general helper that calls
AC_PATH_PROG() to locate an executable and aborts if it is not found.
Andre Noll [Sat, 27 Jan 2018 11:08:04 +0000 (12:08 +0100)]
aft: Avoid implicit fallthrough in switch statement.
The missing return statement causes gcc-7 to complain:
aft.c:2579:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
And indeed, this fall through was not intended when the code in
question was introduced some months ago in commit a7a8d30e (Update
status items on blob events).
The issue is benign though, because also after falling through we
return 0.
Andre Noll [Fri, 29 Dec 2017 14:08:15 +0000 (15:08 +0100)]
play: Print help text if no audio files are given.
This is more helpful than the lopsub error message. Since
handle_help_flags() already checked the arg count, the call to
lls_check_arg_count() can be removed.
Andre Noll [Sun, 14 Jan 2018 21:12:32 +0000 (22:12 +0100)]
play: Always check arg count and init keymap.
Currently we miss to do so if the config file does not exist or
is empty. This triggers the following assertion if, in addition,
no non-option arguments given:
This is because we try to allocate a zero sized buffer in main() due
to lls_num_inputs() returning 0. Fix this by changing the target of
the goto in handle_help_flags().
This bug was introduced during the conversion to lopsub.
Andre Noll [Sun, 31 Dec 2017 14:04:16 +0000 (15:04 +0100)]
Merge branch 'maint'
A conflict in init_random_seed_or_die() of gcrypt.c. The fix added
in the maint branch conflicted against the master branch which also
modified the function to bump the required libgcrypt version.
* maint:
resample filter: Don't discard const.
gcrypt: Seed PRNG in init_random_seed_or_die().
bash_completion: Get rid of weird retry logic.
Andre Noll [Fri, 29 Dec 2017 13:50:28 +0000 (14:50 +0100)]
resample filter: Don't discard const.
Newer versions of libsamplerate made the data_in pointer const. This
causes the following warning:
In file included from resample_filter.c:6:0:
/usr/local/include/samplerate.h:177:6: note: expected 'float *' but argument is of type 'const float *'
void src_short_to_float_array (const short *in, float *out, int len) ;
^
resample_filter.c:173:7: warning: passing argument 1 of 'free' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
In file included from para.h:11:0,
from resample_filter.c:10:
/usr/include/stdlib.h:460:13: note: expected 'void *' but argument is of type 'const float *'
extern void free (void *__ptr) __THROW;
The problem is that we first convert the input from int16 to float
and use the data_in pointer as the target for the conversion.
Fix this by introducing a temporary non-const variable for the
converted input.
Andre Noll [Thu, 16 Nov 2017 01:18:50 +0000 (02:18 +0100)]
crypt.c: Plug memory leak in get_public_key().
If server.users refers to an existing file which is not a ssh public
key, we leak 4 bytes of memory:
==27302== 4 bytes in 1 blocks are definitely lost in loss record 1 of 8
==27302== at 0x402C201: malloc (vg_replace_malloc.c:299)
==27302== by 0x8052FF3: para_malloc (string.c:63)
==27302== by 0x8066532: get_public_key (crypt.c:151)
==27302== by 0x80569D1: user_list_init (user_list.c:90)
==27302== by 0x804D74D: parse_config_or_die (server.c:279)
==27302== by 0x804C719: server_init (server.c:554)
==27302== by 0x804C719: main (server.c:655)
Andre Noll [Thu, 28 Dec 2017 15:48:48 +0000 (16:48 +0100)]
gcrypt: Seed PRNG in init_random_seed_or_die().
The function is supposed to call srandom(3) to set the seed for a new
sequence of pseudo-random integers to be returned by random(3). The
openssl crypto backend does this, but the gcrypt one does not.
This is not a fatal flaw as we don't use random(3) for any
cryptographic purpose. Let's fix it anyway.
Andre Noll [Sun, 5 Nov 2017 18:38:44 +0000 (19:38 +0100)]
bash_completion: Get rid of weird retry logic.
It's totally unclear what this is supposed to do, as the commit which
introduced it six years ago does not say anything about the space
character being the only "proper" value.
Removing the check fixes an issue that only happens on the first
completion attempt: if a unique completion is possible, the completer
prints this completion as if there were multiple completions possible
instead of completing the command line.
Andre Noll [Tue, 8 Aug 2017 11:08:27 +0000 (13:08 +0200)]
server: Disable com_tasks().
This server subcommand will break once we terminate the scheduler
before calling the command handler. The command was only a debugging
aid anyway, and the output was not very interesting to begin with. In
particular, it did not include the tasks of the afs process, and
always showed the same three tasks (command, vss and status).
We keep the subcommand for backward compatibility, but this commit
changes it to be a no-op. Scheduled for removal in v0.7.0.
Andre Noll [Mon, 7 Aug 2017 18:33:55 +0000 (20:33 +0200)]
server: Move afs_pid out of mmd struct.
The afs_pid variable is read-only after it has been initialized. Hence
it does not qualify for being part of the shared memory area which
contains the mmd struct. Make it a global variable instead.
Andre Noll [Sun, 6 Aug 2017 21:45:38 +0000 (23:45 +0200)]
server: Don't pass peername to handle_connect().
The only reason we pass it is that handle_connect() can print a
log message containing the peer name. However, we already log this
information right after the connection was accepted.
Andre Noll [Sun, 6 Aug 2017 14:26:14 +0000 (16:26 +0200)]
vss: Reduce indentation level in vss_send().
If the current chunk is not due yet, we return from the function.
Reversing the logic of the test allows to reduce indentation by
one level. No semantic change.
Andre Noll [Sun, 6 Aug 2017 12:21:15 +0000 (14:21 +0200)]
vss: Propagate return value of afh_get_chunk().
With dynamic chunks, afh_get_chunk() may fail. Currently
vss_get_chunk() prints an error message in this case and returns
the null pointer. However, some callers of vss_get_chunk() happily
dereference the returned pointer without checking for NULL.
This patch modifies vss_get_chunk() to return int and teaches all
callers to check the return value. For the udp and dccp transport,
we disable the fec client temporarily in the error case while for
the http transport we log the error (but otherwise ignore it), and
try to continue with the next chunk.
This flaw was noticed by the clang static analyzer.
Andre Noll [Mon, 14 Aug 2017 00:10:44 +0000 (02:10 +0200)]
fd.c: Change return value of file_exists() to bool.
file_exists() is in fact a misnomer, since it simply calls stat(2),
which may fail for many reasons besides ENOENT. But that's another
issue for another patch..
Andre Noll [Sun, 10 Sep 2017 12:15:50 +0000 (14:15 +0200)]
Shorten copyright notice.
The GPLv2 line does not add any additional information, so drop
it. This leaves a single line of legalese text for most files, which
is about the amount of screen real estate it deserves.
This patch was created with the following script (plus some manual
fixups):
awk '{
if (NR <= 5) {
gs = gensub(/.*Copyright.* ([0-9]+).*Andre Noll.*/, "\\1", "g")
if (gs != $0)
year = gs
next
}
if (NR == 6 && year != "")
printf("/* Copyright (C) %s Andre Noll <maan@tuebingen.mpg.de>, see file COPYING. */\n", year)
print
}'
Andre Noll [Sun, 10 Sep 2017 09:08:14 +0000 (11:08 +0200)]
Merge branch 'refs/heads/t/opus'
This series abstracts out the custom header concept of ogg/vorbis
streams and introduces a public API for it. The new API is employed
for ogg/opus streams to guarantee that fixed size audio format headers
are used for streaming.
Cooking for six weeks.
* refs/heads/t/opus:
opus_afh: Use custom header API to strip comment packet.
ogg: Abstract out custom header code.
Prefix public functions of ogg_afh_common.c with "oac".
afh_recv: Downgrade severity of log message.
Andre Noll [Sun, 3 Sep 2017 08:28:30 +0000 (10:28 +0200)]
Merge branch 'refs/heads/t/portable_io'
A small series with no user-visible impact. After this is merged,
no more casts are necessary for pointers passed to read_u*() and
write_u*() and their big-endian counterparts.
Cooking for five weeks.
* refs/heads/t/portable_io:
blob: Remove some unnecessary casts for arguments to read_u32().
crypt: Remove read_ssh_u32().
Let helpers in portable_io.h receive void * arguments.
Andre Noll [Fri, 11 Aug 2017 10:18:26 +0000 (12:18 +0200)]
audiod: Add log message for receiver errors.
If the receiver could not be opened, we return negative from
open_receiver() and audiod prints an error message which contains the
error string. However, this message is confusing because it does not
say that the error came from open_receiver(), much less which receiver
caused the error. The new log message should clarify this.
Andre Noll [Tue, 11 Jul 2017 14:08:32 +0000 (16:08 +0200)]
Version 2 moods.
This introduces a context-free grammar for moods. The parser for the
corresponding language (the version 2 mood parser) is generated by
flex and bison and will eventually replace the open-coded parser for
traditional (version 1) moods. Those are still supported, but the
server now logs a deprecation warning when a version 1 mood is loaded,
Loading a version 2 mood is a two step procedure. In the first step the
bison parser reads the mood definition stored in the moods table of the
afs database. If there are no errors, the parser returns an abstract
syntax tree which represents the syntactic structure of the mood
definition. In the second step the abstract syntax tree is evaluated
for each row of the audio file table in turn. If the evaluation
function returns true, the audio file is considered admissible. In
this case a reference to the row is added to the score table in the
same way the version 1 mood parser stores the set of admissible files.
The commit adds the following new files to the repository:
* yy/mp.lex: the lexer
* yy/mp.y: the bison parser
* yy/makefile: rules for building the parser, included from main Makefile
* mp.c: frontend (high-level API) and backend (helpers for yy/mp.[ch])
* mp.h: backend data structures and function prototypes
The frontend is only needed in the existing mood.c, which now also
contains the declarations of the frontend API so that mp.h needs only
be included by the lexer and the parser.
The comment at the top of mp.c gives an overview of the mood parser
API. All non-static functions of mp.c (both frontend and backend)
are fully documented. The section on moods of the user manual has
been rewritten and extended.
Since flex and bison are required to build para_server, the list
of optional software packages now mentions these tools, with links
to their home page. If either tool is not installed, the configure
script succeeds but para_server will not be built.
Andre Noll [Sun, 20 Aug 2017 13:54:25 +0000 (15:54 +0200)]
Merge branch 'refs/heads/t/si_update'
A single patch which teaches the afs event handler to trigger a status
item update when the image or lyrics table changes. This patch had a
bug which was only noticed after the branch had been merged to next.
Therefore the series contains a fixup commit.
Cooking for almost two months.
* refs/heads/t/si_update:
server: Avoid NULL pointer dereference in make_status_items().
Update status items on blob events.
Andre Noll [Sun, 30 Jul 2017 00:25:26 +0000 (02:25 +0200)]
Force status item update on mood/playlist changes.
This patch increments the mmd event counter whenever a new playlist
or mood is loaded. This instructs the stat command handlers to resend
the list of status items to all connected stat clients.
Andre Noll [Sun, 30 Jul 2017 00:04:47 +0000 (02:04 +0200)]
afs: Use correct error code for com_select().
If the argument does not start with "m/" or "p/" we have an invalid
argument rather than a syntax error. This removes the last user of
E_AFS_SYNTAX, so the error code can be removed as well.
Andre Noll [Sun, 9 Jul 2017 21:31:43 +0000 (23:31 +0200)]
mood.c: Don't try twice to activate an invalid mood.
(Re-)loading a mood by executing the "select" server command fails
if the mood definition is invalid. In this case we first try to
switch back to the old mood, and if this also fails, fall back to
the dummy mood.
If the mood which is currently active has been replaced by an invalid
mood, switching back to the old mood is pointless because this will
try to load the same invalid mood again.
This commit modifies the select callback to load the old mood only
if it is different from the new mood.
Andre Noll [Sun, 6 Aug 2017 12:05:36 +0000 (14:05 +0200)]
vss: Remove self-made prefaulting.
It used to help a bit on a very old laptop with broken IDE hardware,
see commit 7bba6232 (vss: Mmap audio files using MAP_POPULATE) from
six years ago. But even there it did not eliminate buffer underruns
completely. Our prefault code is too ugly to live, and its benefits
are questionable at best, so let's get rid of it.
We still map the audio file with MAP_POPOLATE, which should be enough
get the full audio file cached before chunks are sent.
Andre Noll [Sat, 5 Aug 2017 16:01:41 +0000 (18:01 +0200)]
build: Prefer AC_CHECK_TYPE() over AC_LINK_IFELSE().
Both invocations of AC_LINK_IFELSE() check if a C structure is
defined. We don't need to run the linker for that, saving a few cycles
on Linux systems where both structures are defined.
Andre Noll [Sun, 6 Aug 2017 13:13:04 +0000 (15:13 +0200)]
wma_afh: Add some assert() statements.
The clang static analyzer seems to have a hard time proving that if
convert_utf8_to_utf16() returns non-negative, the destination pointer
is non-null. This patch should help it.
Andre Noll [Sun, 6 Aug 2017 20:50:15 +0000 (22:50 +0200)]
server: Perform hostname lookup in client process.
When a connection arrives at the TCP command socket, the server process
looks up the remote name before it forks the child process. This is bad
because the server process can not afford to sleep, but name lookup may
take some time, for example if a DNS server is currently unavailable.
We may as well look up the hostname after the fork in the child
process, where blocking operations are not a problem.
Andre Noll [Sun, 6 Aug 2017 19:24:03 +0000 (21:24 +0200)]
server: Fix memory leak in com_check().
parse_mood_line() is called by the select command to set up a mood
item structure for each line of the mood definition. The item is stored
in one of the three lists of the mood structure provided by the caller.
The check command also calls parse_mood_line() to verify the mood
definitions. However, since it does not create a new mood, it does
not allocate a mood structure and passes a null pointer instead.
parse_mood_line() frees the mood item in the error case, but misses
to do so if a successfully parsed mood line was not added to any of
the three lists because the mood pointer is NULL, leaking the mood
item. This commit plugs the leak by adjusting the condition on which
to free the mood item.
This bug was introduced 10 years ago when mood checking was implemented
in commit 02baea14. It was found by code inspection.
Andre Noll [Mon, 7 Aug 2017 17:51:15 +0000 (19:51 +0200)]
server: Rename init_vss_task() to vss_init().
The function not only initializes the vss task but also the three
senders. Moreover, all other public functions of vss.c are also
prefixed with vss_. Finally, the new name is shorter.
Andre Noll [Mon, 7 Aug 2017 21:01:16 +0000 (23:01 +0200)]
server: Fix comment of empty_status_items().
The function is not "used by vss", it is not even visible from vss.c
as it is a static function defined in command.c. De-doxyfy the comment
while at it.
Andre Noll [Mon, 7 Aug 2017 17:17:42 +0000 (19:17 +0200)]
server: Kill two pointless variables in vss.c.
Both variables are set but never used. The two variables were
introduced back in the CVS days and have been pointless for the whole
git history since 2006.
Andre Noll [Fri, 28 Jul 2017 21:13:52 +0000 (23:13 +0200)]
Improve documentation of signal_init_or_die().
Prefix the reference to para_install_sighandler() with \ref to make
sure doxygen warns if the reference becomes stale, and fix the man
section of exit(): it is not a system call (but _exit() is).
Andre Noll [Sun, 30 Jul 2017 21:11:38 +0000 (23:11 +0200)]
stdin/stdout: Only set nonblock flags for non-tty fds.
Although the ->post_select methods of all paraslash executables perform
I/O only when select(2) reports that the file descriptor is ready,
we set the O_NONBLOCK flag for all monitored fds. This is considered
good practice because, in general, it might happen that a subsequent
read(2) call blocks even if select(2) indicates that the fd is ready
for reading. For example, an fd corresponding to a TCP socket might
be flagged as ready for reading if a network packet with incorrect
checksum has arrived, but a subsequent read(2) blocks until the packet
has been retransmitted.
However, stdin and stdout often correspond to a terminal device where
the above scenario won't happen. Moreover, for terminals it's essential
to reset the O_NONBLOCK flag to the old value on exit because the
shell refers to the same file description and thus shares the file
status flags, including O_NONBLOCK. Many terminal applications, for
example dialog(1), expect stdout to be set to blocking mode and fail
in arcane ways if O_NONBLOCK is set.
When the stdin and stdout tasks are about to exit, they reset the
file status flags back to the original values. However if "para_client
stat" is killed with SIGINT, SIGTERM or SIGKILL, or put to sleep with
SIGSTOP, the O_NONBLOCK flag remains set because para_client does not
handle signals at all. para_recv, para_filter and para_write suffer
from the same issue. Adding signal handling to these programs would
not help in the SIGSTOP case because this signal can not be caught.
This patch modifies stdin.c and stdout.c to no longer set O_NONBLOCK
for fd 0 and fd 1 if these fds are associated with a terminal
device. This is much easier and should do the job as well.