On 12/11/22 11:01, Andrew Gregory wrote:
On 11/11/22 at 12:22pm, Andrew Gregory wrote:
On 11/10/22 at 07:01pm, Allan McRae wrote:
On 10/11/22 18:33, Remi Gacogne wrote:
On 10/11/2022 03:58, Allan McRae wrote:
In fact, that whole while loop looks weird to me. Do we need one here? It looks like if the read() call fails, we bail. Then only bail if we processed that call correctly? Weird...
I believe we need the loop because we might have to process more than one callback event. We want to exit the loop as soon as either the read() call failed, or processing one of the even failed (_alpm_sandbox_process_cb_log or _alpm_sandbox_process_cb_download returning false), so we could get rid of the "done" variable by always breaking indeed, since when we do break it is useless to set "done = true".
Great - that is the context I needed to understand the loop. I have made the changes.
If I hear no other comments by the end of the week, I will push this.
My comments are on the gitlab branch.
This needs a decent amount of work, can you make a merge request or send the revised patches to the list?
Thanks for the reviews. I had noticed the -U <url> issue the other night and am trying to figure that out. It falls into the "not easy" category. I can make a merge request, but don't expect to have a chance to deal with some of the comments you made in the next couple of weeks. I'm happy for anyone to provide fix-up patches. Allan