From: Al Viro <viro@zeniv.linux.org.uk>
To: Carmeli Tamir <carmeli.tamir@gmail.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] Use list.h instead of file_system_type next
Date: Sat, 4 May 2019 14:20:08 +0100 [thread overview]
Message-ID: <20190504132008.GY23075@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20190504094549.10021-2-carmeli.tamir@gmail.com>
On Sat, May 04, 2019 at 05:45:48AM -0400, Carmeli Tamir wrote:
> From: Tamir <carmeli.tamir@gmail.com>
>
> Changed file_system_type next field to list_head and refactored
> the code to use list.h functions.
... except that list_head is not a good match here. For one thing,
we never walk that thing backwards. For another, filesystem
can be used without ever going through register_filesystem(),
making your data structure quite a mess - e.g. use of list_empty()
(a perfectly normal list.h primitive) on it might oops on some
instances.
IOW, what you are making is not quite list_head and pretending it
to be list_head is asking for serious headache down the road.
Frankly, what's the point? Reusing an existing data type, to
avoid DIY is generally a good advice, but then you'd better
make sure that existing type *does* fit your needs and that
your creation is playing by that type's rules.
This patch does neither...
NAKed-by: Al Viro <viro@zeniv.linux.org.uk>
next prev parent reply other threads:[~2019-05-04 13:20 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-04 9:45 [PATCH 0/2] Refactor file_systems to use the kernel's list Carmeli Tamir
2019-05-04 9:45 ` [PATCH 1/2] Use list.h instead of file_system_type next Carmeli Tamir
2019-05-04 13:20 ` Al Viro [this message]
2019-05-04 13:45 ` Matthew Wilcox
2019-05-05 18:25 ` Tamir Carmeli
2019-05-06 2:49 ` Matthew Wilcox
2019-05-06 15:16 ` kbuild test robot
2019-05-06 17:33 ` kbuild test robot
2019-05-04 9:45 ` [PATCH 2/2] Changed unsigned param type to unsigned int Carmeli Tamir
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190504132008.GY23075@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=carmeli.tamir@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.