Merge topic branch t/afs-select into master A single patch which silences the select command. The merge conflicted in afs.c, but that was trivial to resolve. * refs/heads/t/afs-select: server: Implement select -verbose.
Merge topic branch t/afs-ls-a into master A new feature for the ls command. Unfortunately, several bugs were found after the topic graduated to next, so the series contains a few fixup commits on top of the single patch which implements the feature. * refs/heads/t/afs-ls-a: afs: Really fix memory leak in mood_load(). afs: Fix memory leak in mood_load(). playlist: Fix error handling of playlist_load(). server: Fix NULL pointer dereference in com_ls(). Implement ls --admissible=m/foo.
afs: Really fix memory leak in mood_load(). The previous fix was insufficient as we also have to destroy the score table in the success case, so move the freeing to destroy_mood(). Fixes: bb0aec0963b1b2da617aebda26deca576684436c
afs: Fix memory leak in mood_load(). In the result != NULL case we open a fresh score table but miss to close it if add_to_score_table() fails. This should never happen, but still.. Fixes: 2d2637cb4c9ab76fea6bc336b9af88fd00bf5e08
server: Fix NULL pointer dereference in com_ls(). The previous commit which extended the -a option of the ls command to accept an optional argument introduced the following flaw: If the argument of -a corresponds to the name of a mood for which no files are admissible, the server crashes due to a NULL pointer dereference because mood_load() leaves the mood instance pointer uninitialized although it returns zero, indicating success. This behaviour of mood_load() contradicts the promises made in its documentation. Fix mood_load() by not special-casing the "zero admissible files" case, which even simplifies the code a bit. If all goes well but no files turn out to be admissible, we now open the score table anyway and set the mood pointer to the allocated mood as usual. Since get_statistics() may now be called with zero admissible files, we have to add a check there before dividing by the number of admissible files, Fixes: 2d2637cb4c9ab76fea6bc336b9af88fd00bf5e08
server: Implement select -verbose. Just set ->pbout to NULL if the new option is not given. This supresses normal output while error messages still make it to the client because those are sent with afs_error().
afs: Fix memory leak in mood_load(). If mood_load() manages to load the mood but does not find any admissible files, it does not deallocate the mood instance and does not set up the global current_mood variable either. Plug the resulting memory leak by destroying the mood also if there are no admissible files (ret == 0).
Implement ls --admissible=m/foo. Currently there can be only one score table at a time because the functions of score.c refer to the global score_table variable. To implement the new feature, we need to overcome this restriction so that the callback of the ls command can populate an independent score table to print its output without interfering with the score table that is currently active. This commit changes most functions of score.c to receive an additional table pointer argument. All current users of the score table pass a NULL pointer to instruct the functions to operate on the global score table as before. However, if the ls command is invoked with an optional mood argument to -a. the callback calls mood_load(), followed by mood_loop() and mood_unload(). The former returns an opaque handle which is then passed to the other two functions to instruct them to operate on the temporary score table instead of the global one. To make the feature work for playlists as well, analogous functionality is implemented in playlist.c. The new mop_loop() of aft.c performs the disambiguation in a similar way as the activate_mood_or_playlist() does. It is a bit simpler though, since the ls command does not have to deal with NULL arguments and does not need to fall back to the dummy mood.
Merge topic branch t/afs-cleanups into master A fair number of patches which clean up all parts of the audio file selector. The most visible change is probably that error messages from afs callbacks are sent with a proper sideband designator so that they are written to stderr on the client side. * refs/heads/t/afs-cleanups: (30 commits) Introduce afs_error(). afs.c: Move com_select() and its callback down. Rename mood_switch(), mood_close(), playlist_{open/close}. Assume that score_open() and score_clear() always succeed. playlist.c: Rename playlist_info -> playlist_instance. mood.c: Rename struct mood to mood_instance. afs.c: Rename ->handler of struct callback_query to ->cb. Simplify and improve activate_mood_or_playlist(). afs: Replace ->init of afs tables by table operations. Merge load_playlist() into playlist_open() and simplify. Simplify row_belongs_to_score_table(). Remove mood.h. Clean up and rename change_current_mood(). mood.c: Simplify and rename load_mood(). mood.c: Move struct statistics into struct mood. afs.c: Improve activate_mood_or_playlist(). Improve playlist_open(). blob.c: Don't initialize table pointer in table->init(). blob: Constify name argument of blob_get_def_by_name(). Rename admissible_file_loop() -> score_loop(). ...
mood: Fix compute_score(). This fixes a bug which was introduced 10 months ago in merge commit 88bf6848d1c (Merge branch 'refs/heads/t/rm_v1_moods'). This merge conflicted in mood.c, and the conflicting hunks that touched compute_score() were resolved incorrectly. Put simply, we kept the old code and disregarded the correction factors that were introduced by the other side of the merge. As a result, at mood load time the correction factors were initialized correctly but not taken into account for computing the score of the admissible files. The fact that the bug went unnoticed for so long can only mean that the correction factors don't make much of a difference in practice. However, the bug was found because one particular mood behaved unexpectedly, likely because its admissible files consisted of a bunch of very new files among many very old ones.
Introduce afs_error(). The callbacks of some afs commands employ the normal ->pbpout para buffer to send an error message to the client on failure. These messages are therefore tagged with the OUTPUT sideband designator just as regular command output. The receiving client writes such messages to stdout, so applications which call para_client have no other way than parsing the output to guess whether it is normal command output or an error message. This commit improves on this by providing a public helper in afs.c to format and send an error message that is tagged with the ERROR sideband designator and thus gets written to stderr on the client side. All afs callbacks which currently use ->pbout for error messages are converted to call the new helper.
Rename mood_switch(), mood_close(), playlist_{open/close}. This naming is unfortunate because we also have the static {mood,pl}_{open,close}() in blob.c which operate on the osl table. In contrast, the functions renamed in this commit operate on blob objects and change the current mood or playlist. Let's call these operations load/unload to avoid confusion.
Assume that score_open() and score_clear() always succeed. Since the score table has only volatile columns, the only possible error is memory exhaustion, in which case we can only abort anyway. This patch changes score_open() to abort if osl_open() fails. This allows us to let score_clear() return void. We can't get rid of the return value of score_open(), however, since a pointer to this function is stored the afs table operations structure.
mood.c: Rename struct mood to mood_instance. A mood is an blob object stored in the mood table. This structure describes something different, so name it accordingly.
Simplify and improve activate_mood_or_playlist(). The logic of this function can be simplified to match the four possible cases for the argument. Each of the four conditional blocks now initializes ret, mode and the message pointer. The message is then printed into the para buffer, which is now passed to the function instead of a char ** because this simplifies the select callback a bit. The other callers (for init and SIGHUP handling) pass NULL and don't need to be adjusted. To make this work we have to make sure that the message pointer is properly initialized in all cases, not only in the error case as before. Thus, playlist_open() and mood_switch() are changed to return a suitable message also on success. For playlists, the message only contains the number of files in the playlist. For moods we also include the afs statistics of the mood and no longer write this information to the server log. We omit the correction factors and the normalization divisor, however, as these are not very interesting. The only purpose of the num_admissible parameter of activate_mood_or_playlist() was to let the caller log this information. This task is now performed by playlist_open() and mood_switch(), so the parameter can be dropped.
Simplify row_belongs_to_score_table(). This function was over-engineered because only one caller passed a non-NULL rank pointer without actually using the rank for anything other than printing it in a log message. So drop the rank parameter and adjust the callers and the log message accordingly. Moreover, the function returned int rather than bool to be able to also return an error code in case the osl lookup function fails. This should never happen though, because the only possible errors are invalid row or table pointers, and these indicate a bug. So abort in this case and let the function return bool.
Remove mood.h. It's too small to be useful. Simply move the three function declarations to afs.h, next to the playlist related functions.
Clean up and rename change_current_mood(). Move the code which destroys the current mood to the end of the function so that we can still return to the old mood if something goes awry. To make this work, various functions need to be adjusted to no longer refer to to afs statistics via the global current_mood pointer. Pass a pointer to the statistics structure to those. Also get rid of the local mood pointer variable in favor of ->m of struct admissible_array. Rename the function because it is public and deserves the mood_ prefix.
mood.c: Simplify and rename load_mood(). We first turn the given mood name into a row of the mood table, then get the mood definition from the row. It's equivalent and much easier to call mood_get_def_by_name() instead. Rename the function because init_mood_parser() tells the reader what the function actually does.
mood.c: Move struct statistics into struct mood. Because it's part of the state of an open mood. Expand and improve the documentation of struct mood and current_mood while at it, and kill the pointless return value of update_afs_statistics().