Erase progress.timer before calling progress.timer_func

Previously, each progress timer function registered with
start_update_progress() was directly used as the timer function of
progress.timer, so it was responsible of erasing the expired timer ID
from that member.  Failing to do this could result in heap corruption.
The progress timer functions normally fulfilled the requirement by
calling update_progress(), but one such function upload_stat_timer()
had to erase the timer ID on its own too.

Now instead, there is a wrapper function progress_timeout(), which
progress.c sets as the timer function of progress.timer.  This wrapper
erases the expired timer ID from progress.timer and then calls the
progress timer function registered with start_update_progress().  So
the progress timer function is no longer responsible of erasing the
timer ID and there's no risk that it could fail to do that in some
error situation.

This commit introduces a new risk though.  Previously, if the struct
progress was freed while the timer was running, the (progress) timer
function would still be called, and it would be able to detect that
the progress pointer is NULL and recover from this situation.  Now,
the timer function progress_timeout() has a pointer to the struct
progress and will dereference that pointer without being able to check
whether the structure has been freed.  Fortunately, done_progress()
asserts that the timer is not running, so this should not occur.

src/network/connection.c

@@ -327,37 +327,26 @@ update_connection_progress(struct connection *conn)
 	update_progress(conn->progress, conn->received, conn->est_length, conn->from);
 }
 
-/** Progress timer callback for @a conn->progress.  As explained in
- * start_update_progress(), this function must erase the expired timer
- * ID from @a conn->progress->timer.  */
+/** Progress timer callback for @a conn->progress.  */
 static void
 stat_timer(struct connection *conn)
 {
 	update_connection_progress(conn);
-	/* The expired timer ID has now been erased.  */
 	notify_connection_callbacks(conn);
 }
 
-/** Progress timer callback for @a conn->upload_progress.  As explained
- * in start_update_progress(), this function must erase the expired timer
- * ID from @a conn->upload_progress->timer.  */
+/** Progress timer callback for @a conn->upload_progress.  */
 static void
 upload_stat_timer(struct connection *conn)
 {
 	struct http_connection_info *http = conn->info;
 
 	assert(conn->http_upload_progress);
-	if_assert_failed return;
 	assert(http);
-	if_assert_failed {
-		conn->http_upload_progress->timer = TIMER_ID_UNDEF;
-		/* The expired timer ID has now been erased.  */
-		return;
-	}
+	if_assert_failed return;
 
 	update_progress(conn->http_upload_progress, http->post.uploaded,
 		http->post.total_upload_length, http->post.uploaded);
-	/* The expired timer ID has now been erased.  */
 	notify_connection_callbacks(conn);
 }
 

src/network/progress.c

@@ -45,8 +45,20 @@ done_progress(struct progress *progress)
 	mem_free(progress);
 }
 
-/* Called from the timer callback of @progress->timer.  This function
- * erases the expired timer ID on behalf of the actual callback.  */
+/** Timer callback for progress.timer.  As explained in install_timer(),
+ * this function must erase the expired timer ID from all variables.  */
+static void
+progress_timeout(void *progress_voidptr)
+{
+	struct progress *const progress = progress_voidptr;
+
+	progress->timer = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
+
+	progress->timer_func(progress->timer_func_data);
+}
+
+/* Usually called from the timer callback of @progress->timer.  */
 void
 update_progress(struct progress *progress, off_t loaded, off_t size, off_t pos)
 {
@@ -89,13 +101,14 @@ update_progress(struct progress *progress, off_t loaded, off_t size, off_t pos)
 		timeval_from_seconds(&progress->estimated_time,
 				   (progress->size - progress->pos) / progress->average_speed);
 
-	install_timer(&progress->timer, SPD_DISP_TIME, progress->timer_func, progress->timer_func_data);
-	/* The expired timer ID has now been erased.  */
+	install_timer(&progress->timer, SPD_DISP_TIME,
+		      progress_timeout, progress);
 }
 
-/* As in @install_timer, @timer_func should erase the expired timer ID
- * from @progress->timer.  The usual way to ensure this is to make
- * @timer_func call @update_progress, which sets a new timer.  */
+/*! Unlike in install_timer(), @a timer_func need not erase the
+ * expired timer ID from @a progress->timer.  update_progress()
+ * installs the timer with a wrapper function that takes care of
+ * erasing the timer ID.  */
 void
 start_update_progress(struct progress *progress, void (*timer_func)(void *),
 		      void *timer_func_data)

src/protocol/bittorrent/connection.c

@@ -186,9 +186,7 @@ update_bittorrent_connection_state(struct connection *conn)
 	}
 }
 
-/* Progress timer callback for @bittorrent->upload_progress.  As
- * explained in @start_update_progress, this function must erase the
- * expired timer ID from @bittorrent->upload_progress->timer.  */
+/* Progress timer callback for @bittorrent->upload_progress.  */
 static void
 update_bittorrent_connection_upload(void *data)
 {
@@ -198,7 +196,6 @@ update_bittorrent_connection_upload(void *data)
 			bittorrent->uploaded,
 			bittorrent->downloaded,
 			bittorrent->uploaded);
-	/* The expired timer ID has now been erased.  */
 }
 
 void