Thanks!

Merged to master at 3d2b33e90036..915eac00a7e0 (from, to]

You can see the entire diff with 'git diff' or at
https://github.com/brho/akaros/compare/3d2b33e90036...915eac00a7e0

Barret


On 2016-01-25 at 11:00 Dan Cross <[email protected]> wrote:
> On Mon, Jan 25, 2016 at 10:38 AM, Barret Rhoden <[email protected]>
> wrote:
> 
> > On Friday, January 22, 2016 at 2:30:37 PM UTC-5, crossd wrote:
> > >
> > > The following changes since commit
> > > 6ae8195b99f28d2f2735dcde2a723a4bde3142ef:
> > >
> > >   Add taps for pipes. (2016-01-14 16:04:46 -0500)
> > >
> > > are available in the git repository at:
> > >
> > >   [email protected]:dancrossnyc/akaros.git slice
> >
> > > From a3cc5dd44b4769b82823a63ce5c4a41de4b45ad5 Mon Sep 17 00:00:00
> > > 2001 From: Dan Cross <[email protected]>
> > > Date: Fri, 22 Jan 2016 14:26:11 -0500
> > > Subject: Slices: A growable list of pointers.
> >
> > > diff --git a/kern/lib/slice.c b/kern/lib/slice.c
> >
> > > +int slice_put(struct slice *slice, size_t i, void *p)
> > > +{
> > > +     if (i >= slice->len)
> > > +             return 0;
> >
> > Minor thing, but can you have success return 0 and failure return
> > non-zero?  (for this function and for all of these in this library).
> > That's the general style we have for functions that return int.
> > Alternative, you can have all of these return bool (TRUE/FALSE) for
> > success.
> >
> 
> Done; changed to use bool. (Incidentally, this means that the values
> didn't actually change, but the intent is much clearer.)
> 
> > +     slice->ptrs[i] = p;
> > > +     return 1;
> > > +}
> > > +
> > > +int slice_del(struct slice *slice, size_t i)
> > > +{
> > > +     if (i >= slice->len)
> > > +             return 0;
> > > +     memmove(slice->ptrs + i, slice->ptrs + i + 1,
> > > +             (slice->len - i + 1) * sizeof(void *));
> >
> > I think there's an off-by one (or two!) with the length calculation.
> > Should it be (slice->len - (i + 1)) or (slice->len - i - 1)?
> > Imagine we have len = 2 and remove the i = 0 item.  The original
> > calculated len was 2 - 0 + 1 = 3.  It should be 1.  Another way to
> > look at it is the +1 was due to the count of all items up to and
> > including i, and that should be subtracted from the total length.
> >
> 
> You are correct; (i + 1) should have been parenthesized.
> 
> > +void *slice_finalize(struct slice *slice)
> >
> > Should this be a void ** for the return type?  I wasn't quite clear
> > on what finalize does.  (remove the list from the slice, return it,
> > and the caller needs to know how long it is?)
> >
> 
> We talked about this; it wasn't clear to me that it was exactly
> legal, but upon closer reading of the standard, I think that 'void
> **' is ok. Changed accordingly.
> 
> > +{
> > > +     void **ps;
> > > +
> > > +     ps = kreallocarray(slice->ptrs, slice->len, sizeof(void *),
> > KMALLOC_WAIT);
> >
> > Is the intent here to shrink the existing allocation?  Our realloc
> > doesn't actually shrink; it will only grow.
> >
> 
> Talked about this face-to-face. Leaving as is for now (our realloc may
> shrink at some point).
> 
> > +     assert(ps != NULL);
> > > +     slice->len = 0;
> > > +     slice->capacity = 0;
> > > +     slice->ptrs = NULL;
> > > +
> > > +     return ps;
> > > +}
> >
> > Barret
> >
> > --
> > You received this message because you are subscribed to the Google
> > Groups "Akaros" group.
> > To unsubscribe from this group and stop receiving emails from it,
> > send an email to [email protected].
> > To post to this group, send email to [email protected].
> > For more options, visit https://groups.google.com/d/optout.
> >
> 

-- 
You received this message because you are subscribed to the Google Groups 
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to