From cabeb58ce69e74c87a1b9fa350bb7ee1d99027d3 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Thu, 15 May 2025 00:27:47 +0200 Subject: [PATCH] aft: Pass correct afsi argument to AFSI_CHANGE event handlers. Each time a new audio file is opened for streaming, we update the numplayed and lastplayed fields of its afs info entry of the audio file table, then update the global afsi and ahfi structures, then trigger an AFSI_CHANGE event, which calls all table event handlers. The event handlers receive as arguments a pointer to the old afsi and a pointer to the already modified aft row. The AFSI_CHANGE event handler of the audio file table gets called in the situation described above, and also via the touch and cpsi subcommands, because these also change the afsi info structure and therefore trigger AFSI_CHANGE events as well. The handler checks whether the affected audio file is the current audio file, and if it is, extracts the modified afsi from the audio file table into the global afsi structure. The problem is that in case of the above "next audio file" scenario, the old afsi pointer points to the global afsi structure, which gets updated by the aft event handler. Therefore the event handlers which happen to run after the event handler of the audio file table see the already updated afsi through the old_afsi pointer. Since the audio file table happens to be the first table of the 7-element afs_tables[] array iterated by afs_event(), all six other event handlers see the incorrect afsi. However, only the moods event handler looks at the old afsi to update the mean and quadratic variation of the lastplayed and numplayed values of the afs statistics. Due to the bug described above, the old and new lastplayed and numplayed values coincide, and therefore the statistics update is a no-op. In practice, at least for moods with many admissible files, the values will only be slightly off, so the bug is rather benign. Fix this by renaming the on-stack new_afsi to old_afsi, adjusting initializations accordingly, and passing a pointer to this instance instead. --- aft.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/aft.c b/aft.c index b10e7bd1..383439d0 100644 --- a/aft.c +++ b/aft.c @@ -1043,7 +1043,7 @@ int open_and_update_audio_file(int *fd) { unsigned char file_hash[HASH2_SIZE]; struct osl_object afsi_obj; - struct afs_info new_afsi; + struct afs_info old_afsi; int ret; struct afsi_change_event_data aced; struct osl_object map, chunk_table_obj; @@ -1108,15 +1108,16 @@ again: ret = -E_HASH_MISMATCH; goto out; } - new_afsi = d->afsi; - new_afsi.num_played++; - new_afsi.last_played = time(NULL); - save_afsi(&new_afsi, &afsi_obj); /* in-place update */ + old_afsi = d->afsi; + d->afsi.num_played++; + d->afsi.last_played = time(NULL); + save_afsi(&d->afsi, &afsi_obj); /* in-place update */ + d->afsi.last_played = old_afsi.last_played; + aced.aft_row = current_aft_row; + aced.old_afsi = &old_afsi; afd.audio_format_id = d->afsi.audio_format_id; load_chunk_table(&afd.afhi, &chunk_table_obj); - aced.aft_row = current_aft_row; - aced.old_afsi = &d->afsi; /* * No need to update the status items as the AFSI_CHANGE event will * recreate them. -- 2.39.5