[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140807160103.GG4856@mwanda>
Date: Thu, 7 Aug 2014 19:01:03 +0300
From: Dan Carpenter <dan.carpenter@...cle.com>
To: Srikrishan Malik <srikrishanmalik@...il.com>, greg@...ah.com,
andreas.dilger@...el.com, oleg.drokin@...el.com,
devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/12] staging: lustre: Fix misplaced opening brace
warnings
On Thu, Aug 07, 2014 at 09:01:36PM +0530, Srikrishan Malik wrote:
> On Wed, Aug 06, 2014 at 11:18:13PM +0300, Dan Carpenter wrote:
> > That looks silly before and after. Everything is indented in a funny
> > way.
>
> Is this better:
>
> static const ldlm_policy_data_t lookup_policy = {
> .l_inodebits = { MDS_INODELOCK_LOOKUP }
> };
>
That is indented too far.
Honestly, I think it looks best on one line but in terms of real life we
can't ignore checkpatch warnings because eventually someone else will
try to "fix" it to not be on one line.
This function has the silly thing where the types are in one column and
the variables are in another. But then over time inevitably we add more
variables and nothing is lined up any more.
I think it's best to move this const variable block to the very front of
the list.
req doesn't need to be initialized.
rc is normally the last variable declared.
lvb_type should be initialized to LVB_T_NONE instead of zero.
__u64 should be u64.
All those changes could be done as one patch titled, "cleanup variable
declarations in mdc_enqueue()". There may be other cleanups you could
do as well. Look hard.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists