[Ocfs2-devel] [PATCH] quota: Remove bogus 'optimization' in check_idq() and check_bdq()

[Ocfs2-devel] [PATCH] quota: Remove bogus 'optimization' in check_idq() and check_bdq()

[Jan Kara]

Checks like <= 0 for an unsigned type do not make much sence. The value
could be only 0 and that does not happen often enough for the check
to be worth it.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/dquot.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/dquot.c b/fs/dquot.c
index 758bf4a..e95ad55 100644
--- a/fs/dquot.c
+++ b/fs/dquot.c
@@ -1041,7 +1041,7 @@ static inline char ignore_hardlimit(struct dquot *dquot)
 static int check_idq(struct dquot *dquot, qsize_t inodes, char *warntype)
 {
 	*warntype = QUOTA_NL_NOWARN;
-	if (inodes <= 0 || test_bit(DQ_FAKE_B, &dquot->dq_flags))
+	if (test_bit(DQ_FAKE_B, &dquot->dq_flags))
 		return QUOTA_OK;
 
 	if (dquot->dq_dqb.dqb_ihardlimit &&
@@ -1073,7 +1073,7 @@ static int check_idq(struct dquot *dquot, qsize_t inodes, char *warntype)
 static int check_bdq(struct dquot *dquot, qsize_t space, int prealloc, char *warntype)
 {
 	*warntype = QUOTA_NL_NOWARN;
-	if (space <= 0 || test_bit(DQ_FAKE_B, &dquot->dq_flags))
+	if (test_bit(DQ_FAKE_B, &dquot->dq_flags))
 		return QUOTA_OK;
 
 	if (dquot->dq_dqb.dqb_bhardlimit &&
-- 
1.5.2.4

[Ocfs2-devel] [PATCH] quota: Remove bogus 'optimization' in check_idq() and check_bdq()

[Joel Becker]

On Mon, Oct 20, 2008 at 07:23:36PM +0200, Jan Kara wrote:
> Checks like <= 0 for an unsigned type do not make much sence. The value
> could be only 0 and that does not happen often enough for the check
> to be worth it.

	Why not worth it?  Certainly the '<' isn't valid, but is it even
possible to call these functions with 0?  I don't know enough to answer
that, but I'm wondering if it should BUG_ON(inodes == 0).  If not, what
happens after this change?  Obviously inodes==0 means the quota doesn't
change, but could they get a soft warning they wouldn't get before?

Joel

-- 

Life's Little Instruction Book #222

	"Think twice before burdening a friend with a secret."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH] quota: Remove bogus 'optimization' in check_idq() and check_bdq()

[Jan Kara]

On Tue 21-10-08 10:29:57, Joel Becker wrote:
> On Mon, Oct 20, 2008 at 07:23:36PM +0200, Jan Kara wrote:
> > Checks like <= 0 for an unsigned type do not make much sence. The value
> > could be only 0 and that does not happen often enough for the check
> > to be worth it.
> 
> 	Why not worth it?  Certainly the '<' isn't valid, but is it even
> possible to call these functions with 0?  I don't know enough to answer
> that, but I'm wondering if it should BUG_ON(inodes == 0).  If not, what
> happens after this change?  Obviously inodes==0 means the quota doesn't
> change, but could they get a soft warning they wouldn't get before?
  In theory, yes, user would now get a warning when he previously did not
get it. But I would consider that to be an advantage (warning is issued at
most once and it could happen that warning is not issued yet only because
of some unusual reason like root chowned a file to the user and got him
over quota)... Anyway I'm not aware of a place which would actually call the
code with 0 so I think this is mostly academic question :).
  Anyway, thanks for comments.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR