Hi -

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.

> +     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.


> +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?)

> +{
> +     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.  

> +     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.

Reply via email to