Messages in this thread | | | Date | Mon, 2 Jun 2008 19:41:46 +0200 | From | "Vegard Nossum" <> | Subject | Re: [PATCH] quota: Remove use of info_any_dirty() |
| |
Hi,
On Mon, Jun 2, 2008 at 7:11 PM, Jan Kara <jack@suse.cz> wrote: > Since there is only a single place which uses info_any_dirty() and that > is a trivial macro, just remove the use of this macro completely. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/quota.c | 7 +++++-- > include/linux/quota.h | 2 -- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/fs/quota.c b/fs/quota.c > index db1cc9f..f0702f4 100644 > --- a/fs/quota.c > +++ b/fs/quota.c > @@ -199,8 +199,11 @@ restart: > list_for_each_entry(sb, &super_blocks, s_list) { > /* This test just improves performance so it needn't be reliable... */ > for (cnt = 0, dirty = 0; cnt < MAXQUOTAS; cnt++) > - if ((type == cnt || type == -1) && sb_has_quota_enabled(sb, cnt) > - && info_any_dirty(&sb_dqopt(sb)->info[cnt])) > + if ((type == cnt || type == -1) > + && sb_has_quota_enabled(sb, cnt) > + && (info_dirty(&sb_dqopt(sb)->info[cnt]) > + || !list_empty(&sb_dqopt(sb)-> > + info[cnt].dqi_dirty_list))) > dirty = 1;
This is really too hideous in my opinion and looks like a candidate for its own static inline function.
Or you can try to rewrite the boolean expression on multiple lines using continue, something like:
- for (cnt = 0, dirty = 0; cnt < MAXQUOTAS; cnt++) - if ((type == cnt || type == -1) && sb_has_quota_enabled( - && info_any_dirty(&sb_dqopt(sb)->info[cnt])) - dirty = 1; + for (cnt = 0, dirty = 0; cnt < MAXQUOTAS; cnt++) { + if (type != cnt && type != -1) + continue; + if (!sb_has_quota_enabled(sb, cnt)) + continue; + if (!info_any_dirty(&sb_dqopt(sb)->info[cnt])) + continue; + dirty = 1; + }
(This uses the original macro, I know. How about moving that from the header to a new inline function just above this function?)
What do you think?
Vegard
PS: This is a really good clean-up effort. Good work!
-- "The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
| |