Hello,

On Sun, Sep 12, 2021 at 1:24 PM Mark Wielaard <mark@klomp.org> wrote:
> The escaped_string is created outside the loop and reused each time
> (good). But...
> > +
> >    /* Initialize each handle.  */
> >    for (int i = 0; i < num_urls; i++)
> >      {
> > @@ -904,16 +908,23 @@ debuginfod_query_server (debuginfod_client *c,
> >        if (filename) /* must start with / */
> >          {
> >            /* PR28034 escape characters in completed url to %hh format. */
> > -          char *escaped_string;
> > -          escaped_string = curl_easy_escape(data[i].handle, filename, 0);
> > +          char *loc = escaped_string;
> >            if (!escaped_string)
> >              {
> >                rc = -ENOMEM;
> >                goto out2;
> >              }
>
> This check, and...
>
> > +
> > +          size_t escaped_strlen = strlen(escaped_string);
> > +          while ((loc = strstr(loc, "%2F")))
> > +            {
> > +                loc[0] = '/';
> > +                // pull the string back after replacement
> > +                memmove(loc+1, loc+3,escaped_strlen - (loc - escaped_string + 2) );
> > +                escaped_strlen -=2;
> > +            }
>
> the manipulation of the escaped_string, could both also be done
> outside the loop, since they always do the same thing.
Fixed.

>
> I think you should simply add a new ChangeLog entry at the top instead
> of changing an old existing one. And please do mention the Makefile.am
> (TESTS) and (EXTRA_DIST) addition.
Should be good.

> OK, that is certainly a file name with lots of unexpected characters :)
Gotta be sure :)

If there are any more efficiency changes, ChangeLog changes, or other
changes, don't be afraid to reach out again.

Thanks,

Noah Sanci