On Wed, Apr 11, 2012 at 3:14 PM, Henning Brauer
<lists-openbsdt...@bsws.de> wrote:
> * patrick keshishian <sids...@boxsoft.com> [2012-04-11 14:55]:
>> On Wed, Apr 11, 2012 at 12:20:30PM +0200, Henning Brauer wrote:
>> don't you need two different index vars for this next
>> section?
>
> no, why?
I put the caveat that I am not familiar with the code (and its use).
So apologies if I'm making grave assumptions on the use case (more
below).
>> > + for (i = 0; i < n; i++)
>> > + if (i < npflogifs)
>> > + p[i] = pflogifs[i];
>> > + else
>> > + p[i] = NULL;
>
> i think that is pretty clear: each slot in the newly allocated p gets
> the same value as it had in the old pflogifs, once we're at the end of
> pflogifs we set the remaining slots to NULL. unused slots were NULL
> before so just inheriting the NULL is safe.
Unless pflog_clone_destroy() takes out one in the middle of the list.
I probably assumed too much.
>> something like the following with caveats that a) it is
>> 5am-ish for me and b) i did not try compiling it:
>>
>> for (i = 0, j = 0; i < n; i++, j++) {
>> for (; j < npflogifs && NULL == pflogifs[j]; j++)
>> ;
>> if (j == npflogifs)
>> break;
>> p[i] = pflogifs[j];
>> }
>> for (; i < n; i++)
>> p[i] = NULL;
>
> i gave up following this after a bit.
The loop is like yours, but looks out for an NULL-ed out pflogifs
entry (from pflog_clone_destroy()?). If one is detected, adjust index
into pflogifs accordingly.
Now, if it is the case that pflog_clone_destroy() won't ever take out
an entry in the middle of pflogifs, then ignore my comments.
>> > +
>> > + if(pflogifs)
>> ^^ nit
>
> fixed
>
>> > s = splnet();
>> > pflogifs[pflogif->sc_unit] = NULL;
>> > LIST_REMOVE(pflogif, sc_list);
>> > +
>> > + for (i = npflogifs; i > 0 && pflogifs[i - 1] != NULL; i--)
>> > + ; /* nothing */
>> > + if (i < npflogifs)
>> > + pflogifs_resize(i); /* error harmless here */
>>
>> So, if the last pflogifs entry is NULL don't resize
>> down? Not really questioning the logic, but want to
>> make sure I understand that's what is meant, cause
>> there is an easier check for that than the for()-loop.
>> Caveats: a) 5am-ish, b) not familiar with code.
>
> walk the array backwards until we find the first non-empty slot, then
> shrink it to that.
OK. So the _destroy() code will always take out entries from the end
of the pflogifs array.
Sorry for the noise.
--patrick
> --
> Henning Brauer, h...@bsws.de, henn...@openbsd.org
> BS Web Services, http://bsws.de, Full-Service ISP
> Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully
Managed
> Henning Brauer Consulting, http://henningbrauer.com/
