Hello Raphael, (reordered quotes ahead) On Mon, Apr 16, 2012 at 5:59 AM, Raphael Geissert <geissert@debian.org> wrote: > Comments? > > I'd really like to see it included some time soon. Thanks in advance. Sorry for the delay. We were focused on pushing 0.9 out of the door and in to testing which is finally done as of speaking now… *sound of relieve* I see no big problem with not including it for wheezy so far. > Attached version sort of restores the redirection limits. It adds a new > setting that limits the number of queues that may exist for a given method > that uses a QueueHost mode. Whenever the limit is reached, only one more > queue will be created: one without the host name (as in QueueAccess mode). You have set the limit to 10. Is there some reason for that? By default a normal user will have at least three queues (debian, security, backports). That should be enough and the consequences if not aren't really noticeable anyway, i am just interesting in finding out why exactly 10… Is it because of redirection loop detection (the one in a method is also 10)? Either way, this makes it around 100 redirections before a loop is detected. (or to be corroct: redirects aren't followed anymore regardless of looping) That are not exactly a few and should be quiet noticeable for a user, but as it is an error condition anyway it should be okay. > Under the normal circumstances of a loop on the same host, the loop will > still be caught at the method process. Under a loop of external > redirections, it will take a little longer to detect it, but it will > eventually be found. "Longer" just means three times on the same uri instead of two, right? (A -> B -> A [A.stop set] -> B [B.stop set] -> A [stop seen] -> failure) Shouldn't be a problem, just want to confirm that we are talking about the same or if this hints at something i haven't noticed so far. > [patch itself] > unsigned long QueueHostLimit; Such a change would be an abi break, but we don't need that as we could use as private struct for which we have already reserved a d-pointer or we could just read the value in the only place we accessing it directly. For simplicity i have just done the later in the attached patch. > void pkgAcqMethod::Redirect(const string &NewURI) […] > Queue = Queue->Next; >- I->Next->Next = 0; >- if (QueueBack == 0) >- QueueBack = I->Next; >+ delete Tmp; >+ if (Tmp == QueueBack) >+ QueueBack = Queue; deleting Tmp and after that comparing it with QueueBack looks strange (== like accessing an invalid pointer). But this seems like a copy&paste error given that the same code is used twice in acquire-method.cc. Just added a small helpermethod because of that to take care of dequeue. Patch attached for simplicity. If you are fine with my changes and if i am not running into problems with it over the week you should find it included in the next upload. Best regards David Kalnischkies
Attachment:
apt-dequeuing-accesses-deleted-pointer.diff
Description: Binary data
Attachment:
apt-668111-handle-redirect-in-worker.diff
Description: Binary data