lkml.org 
[lkml]   [2018]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/6] rhashtable: improve documentation for rhashtable_walk_peek()
    On Tue, Mar 27, 2018 at 10:33:04AM +1100, NeilBrown wrote:
    > The documentation for rhashtable_walk_peek() wrong. It claims to
    > return the *next* entry, whereas it in fact returns the *previous*
    > entry.
    > However if no entries have yet been returned - or if the iterator
    > was reset due to a resize event, then rhashtable_walk_peek()
    > *does* return the next entry, but also advances the iterator.
    >
    > I suspect that this interface should be discarded and the one user
    > should be changed to not require it. Possibly this patch should be
    > seen as a first step in that conversation.
    >
    > This patch mostly corrects the documentation, but does make a
    > small code change so that the documentation can be correct without
    > listing too many special cases. I don't think the one user will
    > be affected by the code change.
    >
    > Signed-off-by: NeilBrown <neilb@suse.com>

    We should cc Tom Herbert too since he wrote this code.

    > ---
    > lib/rhashtable.c | 17 +++++++++++++----
    > 1 file changed, 13 insertions(+), 4 deletions(-)
    >
    > diff --git a/lib/rhashtable.c b/lib/rhashtable.c
    > index 3825c30aaa36..24a57ca494cb 100644
    > --- a/lib/rhashtable.c
    > +++ b/lib/rhashtable.c
    > @@ -853,13 +853,17 @@ void *rhashtable_walk_next(struct rhashtable_iter *iter)
    > EXPORT_SYMBOL_GPL(rhashtable_walk_next);
    >
    > /**
    > - * rhashtable_walk_peek - Return the next object but don't advance the iterator
    > + * rhashtable_walk_peek - Return the previously returned object without advancing the iterator
    > * @iter: Hash table iterator
    > *
    > - * Returns the next object or NULL when the end of the table is reached.
    > + * Returns the last object returned, or NULL if no object has yet been returned.
    > + * If the previously returned object has since been removed, then some other arbitrary
    > + * object maybe returned, or possibly NULL will be returned. In that case, the
    > + * iterator might be advanced.
    > *
    > * Returns -EAGAIN if resize event occurred. Note that the iterator
    > - * will rewind back to the beginning and you may continue to use it.
    > + * will rewind back to the beginning and rhashtable_walk_next() should be
    > + * used to get the next object.
    > */
    > void *rhashtable_walk_peek(struct rhashtable_iter *iter)
    > {
    > @@ -880,7 +884,12 @@ void *rhashtable_walk_peek(struct rhashtable_iter *iter)
    > * the table hasn't changed.
    > */
    > iter->skip--;
    > - }
    > + } else
    > + /* ->skip is only zero after rhashtable_walk_start()
    > + * or when the iterator is reset. In this case there
    > + * is no previous object to return.
    > + */
    > + return NULL;
    >
    > return __rhashtable_walk_find_next(iter);
    > }
    >
    >

    --
    Email: Herbert Xu <herbert@gondor.apana.org.au>
    Home Page: http://gondor.apana.org.au/~herbert/
    PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

    \
     
     \ /
      Last update: 2018-03-27 17:31    [W:4.297 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site