[pacman-dev] [PATCH] Cleanup the old sequential download code

[Anatol Pomozov]

Hi

On Wed, Jun 10, 2020 at 8:12 PM Allan McRae <allan at archlinux.org> wrote:
>
> On 12/5/20 3:16 am, Anatol Pomozov wrote:
> > All users of _alpm_download() have been refactored to the new API.
> > It is time to remove the old _alpm_download() functionality now.
> >
> > This change also removes obsolete SIGPIPE signal handler functionality
> > (this is a leftover from libfetch days).
> >
> > Signed-off-by: Anatol Pomozov <anatol.pomozov at gmail.com>
> > ---
> >  lib/libalpm/dload.c | 323 +-------------------------------------------
> >  lib/libalpm/dload.h |   4 -
> >  2 files changed, 3 insertions(+), 324 deletions(-)
> >
> > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> > index 43fe9847..4dbb011f 100644
> > --- a/lib/libalpm/dload.c
> > +++ b/lib/libalpm/dload.c
> > @@ -78,10 +78,6 @@ enum {
> >  };
> >
> >  static int dload_interrupted;
> > -static void inthandler(int UNUSED signum)
> > -{
> > -     dload_interrupted = ABORT_SIGINT;
> > -}
> >
> >  static int dload_progress_cb(void *file, curl_off_t dltotal, curl_off_t dlnow,
> >               curl_off_t UNUSED ultotal, curl_off_t UNUSED ulnow)
> > @@ -236,8 +232,7 @@ static size_t dload_parseheader_cb(void *ptr, size_t size, size_t nmemb, void *u
> >       return realsize;
> >  }
> >
> > -static void curl_set_handle_opts(struct dload_payload *payload,
> > -             CURL *curl, char *error_buffer)
> > +static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload)
> >  {
>
> This change seems beyond the scope for this patch.  Can you split that
> and the other related changes below into a separate patch.


Inlining "error_buffer" is related to the "serial download" code cleanup.

The old (serial) codepath has been using a stack variable error_buffer
and was passing it to curl_set_handle_opts() function as a parameter.
With the concurrent download we cannot share a stack variable between
all payloads, instead we initialize a separate buffer for each
payload. Now "error_buffer is part of dload_payload{}.

With this serial codepath removed we don't have any local variable for
error_buffer. And we don't need to pass "error_buffer" to
curl_set_handle_opts() - "error_buffer" is already part of *payload
parameter.

This parameter inlining organically belongs to this refactoring IMO.