[PATCH 0254/1285] Replace numeric parameter like 0444 with macro

[PATCH 0254/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 drivers/infiniband/hw/cxgb4/cm.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index a3a6721..e2d150a 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -69,73 +69,73 @@ static char *states[] = {
 };
 
 static int nocong;
-module_param(nocong, int, 0644);
+module_param(nocong, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(nocong, "Turn of congestion control (default=0)");
 
 static int enable_ecn;
-module_param(enable_ecn, int, 0644);
+module_param(enable_ecn, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(enable_ecn, "Enable ECN (default=0/disabled)");
 
 static int dack_mode = 1;
-module_param(dack_mode, int, 0644);
+module_param(dack_mode, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(dack_mode, "Delayed ack mode (default=1)");
 
 uint c4iw_max_read_depth = 32;
-module_param(c4iw_max_read_depth, int, 0644);
+module_param(c4iw_max_read_depth, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(c4iw_max_read_depth,
 		 "Per-connection max ORD/IRD (default=32)");
 
 static int enable_tcp_timestamps;
-module_param(enable_tcp_timestamps, int, 0644);
+module_param(enable_tcp_timestamps, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(enable_tcp_timestamps, "Enable tcp timestamps (default=0)");
 
 static int enable_tcp_sack;
-module_param(enable_tcp_sack, int, 0644);
+module_param(enable_tcp_sack, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(enable_tcp_sack, "Enable tcp SACK (default=0)");
 
 static int enable_tcp_window_scaling = 1;
-module_param(enable_tcp_window_scaling, int, 0644);
+module_param(enable_tcp_window_scaling, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(enable_tcp_window_scaling,
 		 "Enable tcp window scaling (default=1)");
 
 int c4iw_debug;
-module_param(c4iw_debug, int, 0644);
+module_param(c4iw_debug, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(c4iw_debug, "Enable debug logging (default=0)");
 
 static int peer2peer = 1;
-module_param(peer2peer, int, 0644);
+module_param(peer2peer, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(peer2peer, "Support peer2peer ULPs (default=1)");
 
 static int p2p_type = FW_RI_INIT_P2PTYPE_READ_REQ;
-module_param(p2p_type, int, 0644);
+module_param(p2p_type, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(p2p_type, "RDMAP opcode to use for the RTR message: "
 			   "1=RDMA_READ 0=RDMA_WRITE (default 1)");
 
 static int ep_timeout_secs = 60;
-module_param(ep_timeout_secs, int, 0644);
+module_param(ep_timeout_secs, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(ep_timeout_secs, "CM Endpoint operation timeout "
 				   "in seconds (default=60)");
 
 static int mpa_rev = 2;
-module_param(mpa_rev, int, 0644);
+module_param(mpa_rev, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(mpa_rev, "MPA Revision, 0 supports amso1100, "
 		"1 is RFC5044 spec compliant, 2 is IETF MPA Peer Connect Draft"
 		" compliant (default=2)");
 
 static int markers_enabled;
-module_param(markers_enabled, int, 0644);
+module_param(markers_enabled, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(markers_enabled, "Enable MPA MARKERS (default(0)=disabled)");
 
 static int crc_enabled = 1;
-module_param(crc_enabled, int, 0644);
+module_param(crc_enabled, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(crc_enabled, "Enable MPA CRC (default(1)=enabled)");
 
 static int rcv_win = 256 * 1024;
-module_param(rcv_win, int, 0644);
+module_param(rcv_win, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(rcv_win, "TCP receive window in bytes (default=256KB)");
 
 static int snd_win = 128 * 1024;
-module_param(snd_win, int, 0644);
+module_param(snd_win, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(snd_win, "TCP send window in bytes (default=128KB)");
 
 static struct workqueue_struct *workq;
-- 
2.9.2

RE: [PATCH 0254/1285] Replace numeric parameter like 0444 with macro

[Steve Wise]

Acked-by: Steve Wise <swise@opengridcomputing.com>

Re: [PATCH 0254/1285] Replace numeric parameter like 0444 with macro

[Steven Rostedt]

On Tue, Aug 02, 2016 at 09:12:54AM -0500, Steve Wise wrote:
> Acked-by: Steve Wise <swise@opengridcomputing.com>
> 

I have to ask, why did you ack all these? There's several things wrong with
this patch series, but even the point of the patch is mistaken. It makes
readable code much less readable. When you chmod a file, do you type

  chmod 0444 file

or do you write

  chmod S_IRUSR|S_IRGRP|S_IROTH file

?

Which of the above is easier to figure what is being changed?

Not to mention, because the subject is the same for all 1285 patches, and you
deleted the content of the patch in your ack, there's no way to know what
exactly this ack is for (I haven't received the original patch yet because
it's probably being ratelimited by some mail server).

-- Steve

RE: [PATCH 0254/1285] Replace numeric parameter like 0444 with macro

[Steve Wise]

> 
> On Tue, Aug 02, 2016 at 09:12:54AM -0500, Steve Wise wrote:
> > Acked-by: Steve Wise <swise@opengridcomputing.com>
> >
> 
> I have to ask, why did you ack all these? There's several things wrong with
> this patch series, but even the point of the patch is mistaken. It makes
> readable code much less readable. When you chmod a file, do you type
> 
>   chmod 0444 file
> 
> or do you write
> 
>   chmod S_IRUSR|S_IRGRP|S_IROTH file
> 
> ?
> 
> Which of the above is easier to figure what is being changed?
>

I assumed this was some global "fix up".   
 
> Not to mention, because the subject is the same for all 1285 patches, and you
> deleted the content of the patch in your ack, there's no way to know what
> exactly this ack is for (I haven't received the original patch yet because
> it's probably being ratelimited by some mail server).
> 

I acked just the single patches that hit cxgb3/cxgb4.  But if this is really
garbage, then ignore my ACKs...

Steve.

Re: [PATCH 0254/1285] Replace numeric parameter like 0444 with macro

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]

On Tue, 2016-08-02 at 10:20 -0500, Steve Wise wrote:
> > 
> > 
> > On Tue, Aug 02, 2016 at 09:12:54AM -0500, Steve Wise wrote:
> > > 
> > > Acked-by: Steve Wise <swise@opengridcomputing.com>
> > > 
> > 
> > I have to ask, why did you ack all these? There's several things
> > wrong with
> > this patch series, but even the point of the patch is mistaken. It
> > makes
> > readable code much less readable. When you chmod a file, do you
> > type
> > 
> >   chmod 0444 file
> > 
> > or do you write
> > 
> >   chmod S_IRUSR|S_IRGRP|S_IROTH file
> > 
> > ?
> > 
> > Which of the above is easier to figure what is being changed?
> > 
> 
> I assumed this was some global "fix up".   

It is, but I'm not so sure I don't agree with Steve.  I'm not sure this
actually makes things better.  At a minimum, I would argue that Bart's
fix is mandatory before they would get my ack.  I would also request
that even though the patches are split up for review, they be squashed
on commit (whoever would want to tackle this monstrous pile of dubious
janitorial stuff).

> > 
> > Not to mention, because the subject is the same for all 1285
> > patches, and you
> > deleted the content of the patch in your ack, there's no way to
> > know what
> > exactly this ack is for (I haven't received the original patch yet
> > because
> > it's probably being ratelimited by some mail server).
> > 
> 
> I acked just the single patches that hit cxgb3/cxgb4.  But if this is
> really
> garbage, then ignore my ACKs...
> 
> Steve.
> 

-- 
Doug Ledford <dledford@redhat.com>
              GPG KeyID: 0E572FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]