Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1288597

Re: [PATCH 1/4] list: introduce list_is_first()

From Josh Poimboeuf <jpoimboe@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH 1/4] list: introduce list_is_first()
Date 2015-12-10 16:30 +0100
Message-ID <qEbkm-7Z2-37@gated-at.bofh.it> (permalink)
References <qEaeC-7jl-25@gated-at.bofh.it> <qEbaF-7V7-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Dec 10, 2015 at 08:10:34AM -0700, Jens Axboe wrote:
> On 12/10/2015 07:17 AM, Geliang Tang wrote:
> >We already have list_is_last(), it makes sense to also add
> >list_is_first() for consistency. This list utility function
> >to check for first element in a list.
> 
> Honestly, I think we already have way too many of these kind of helpers.
> IMHO they don't really help, they hurt readability. You should know how the
> list works anyway, and if you do, then it's a no-brainer what's first and
> last. If you don't, then you are bound to screw up in other ways.
> 
> Just my 2 cents.

Personally I would disagree.  Something like:

  if (list_is_first(&rq->queuelist, &nd->queue))

is much more readable to me than:

  if (rq->queuelist.prev == &nd->queue)

The first one takes no effort for me -- it's almost English.  While the
second one takes me a few seconds (and some precious brain cycles) to
decipher.

Maybe whether it's readable depends on how many years you've been
looking at the pattern.  But IMHO we shouldn't make "having x # of years
staring at kernel code" a prerequisite for being able to read kernel
code.

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 1/4] list: introduce list_is_first() Geliang Tang <geliangtang@163.com> - 2015-12-10 15:30 +0100
  Re: [PATCH 1/4] list: introduce list_is_first() Jens Axboe <axboe@kernel.dk> - 2015-12-10 16:20 +0100
    Re: [PATCH 1/4] list: introduce list_is_first() Josh Poimboeuf <jpoimboe@redhat.com> - 2015-12-10 16:30 +0100
      Re: [PATCH 1/4] list: introduce list_is_first() Christoph Hellwig <hch@infradead.org> - 2015-12-10 16:40 +0100
      Re: [PATCH 1/4] list: introduce list_is_first() Jens Axboe <axboe@kernel.dk> - 2015-12-10 16:40 +0100
        Re: [PATCH 1/4] list: introduce list_is_first() Jiri Kosina <jikos@kernel.org> - 2015-12-10 17:50 +0100

csiph-web