[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[Joe Perches]

On Thu, 2018-02-01 at 09:16 +0530, Viresh Kumar wrote:
> On Thu, Feb 1, 2018 at 9:15 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> > Right but lets no go changing to octals everywhere, new code sure :)
> > I don't see plans to remove representations entirely from kernel yet.
> 
> Sure. I was just pointing out to the discussion where the *preferred* thing
> happened.

Not really true.  Linus prefers actual octal
and suggested treewide conversions.

https://lwn.net/Articles/696229/
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[Viresh Kumar]

On Thu, Feb 1, 2018 at 9:15 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> Right but lets no go changing to octals everywhere, new code sure :)
> I don't see plans to remove representations entirely from kernel yet.

Sure. I was just pointing out to the discussion where the *preferred* thing
happened.
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[Vinod Koul]

On Wed, Jan 31, 2018 at 03:31:48PM +0530, Viresh Kumar wrote:
> On Mon, Jan 29, 2018 at 10:18 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
> >> Current coding style prefers octal permissions values. This patch
> >> changes symbolic permissions to octal values.
> >
> > Is this preference documented anywhere?
> 
> This is where it started.
> 
> lkml.kernel.org/r/CA+55aFzu59sXo7qA8CoCo+tSJgEkW7fWDjkt_tsTzPWZ7gxj1A@mail.gmail.com

Right but lets no go changing to octals everywhere, new code sure :)
I don't see plans to remove representations entirely from kernel yet.

[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[Viresh Kumar]

On Mon, Jan 29, 2018 at 10:18 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
>> Current coding style prefers octal permissions values. This patch
>> changes symbolic permissions to octal values.
>
> Is this preference documented anywhere?

This is where it started.

lkml.kernel.org/r/CA+55aFzu59sXo7qA8CoCo+tSJgEkW7fWDjkt_tsTzPWZ7gxj1A@mail.gmail.com
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[Vinod Koul]

On Mon, Jan 29, 2018 at 06:26:40AM +0000, Yang, Shunyong wrote:
> Hi, Vinod
> 
> On Mon, 2018-01-29 at 10:18 +0530, Vinod Koul wrote:
> > On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
> > > 
> > > Current coding style prefers octal permissions values. This patch
> > > changes symbolic permissions to octal values.
> > Is this preference documented anywhere?
> > 
> 
> When using symbolic permissions like "S_IRUGO | S_IWUSR". The
> checkpatch.pl will output some warnings to suggest to use octal values.
> I quote following lines from checkpatch.pl,
> 
> # check for uses of S_<PERMS> that could be octal for readability
> ...
> if (WARN("SYMBOLIC_PERMS",
>                                  "Symbolic permissions '$oval' are not
> preferred. Consider using octal permissions '$octal'.\n" . $herecurr)
> &&

It is a warning and a preference. And I don't prefer changing code for no
benefit. Sorry but I am not considering this change

[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[Yang Shunyong]

Hi, Vinod

On Mon, 2018-01-29 at 10:18 +0530, Vinod Koul wrote:
> On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
> > 
> > Current coding style prefers octal permissions values. This patch
> > changes symbolic permissions to octal values.
> Is this preference documented anywhere?
> 

When using symbolic permissions like "S_IRUGO | S_IWUSR". The
checkpatch.pl will output some warnings to suggest to use octal values.
I quote following lines from checkpatch.pl,

# check for uses of S_<PERMS> that could be octal for readability
...
if (WARN("SYMBOLIC_PERMS",
                                 "Symbolic permissions '$oval' are not
preferred. Consider using octal permissions '$octal'.\n" . $herecurr)
&&
                            $fix) {
                                $fixed[$fixlinenr] =~ s/$val/$octal/;
                        }

Thanks.
Shunyong
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[Vinod Koul]

On Mon, Jan 22, 2018 at 06:44:40PM +0800, Yang Shunyong wrote:
> Current coding style prefers octal permissions values. This patch
> changes symbolic permissions to octal values.

Is this preference documented anywhere?

[1/2] dmaengine: dmatest: change symbolic permissions to octal values

[Yang Shunyong]

Current coding style prefers octal permissions values. This patch
changes symbolic permissions to octal values.

Signed-off-by: Yang Shunyong <shunyong.yang@hxt-semitech.com>
---
 drivers/dma/dmatest.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index ec5f9d2..4cdccc3 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -24,60 +24,58 @@
 #include <linux/wait.h>
 
 static unsigned int test_buf_size = 16384;
-module_param(test_buf_size, uint, S_IRUGO | S_IWUSR);
+module_param(test_buf_size, uint, 0644);
 MODULE_PARM_DESC(test_buf_size, "Size of the memcpy test buffer");
 
 static char test_channel[20];
-module_param_string(channel, test_channel, sizeof(test_channel),
-		S_IRUGO | S_IWUSR);
+module_param_string(channel, test_channel, sizeof(test_channel), 0644);
 MODULE_PARM_DESC(channel, "Bus ID of the channel to test (default: any)");
 
 static char test_device[32];
-module_param_string(device, test_device, sizeof(test_device),
-		S_IRUGO | S_IWUSR);
+module_param_string(device, test_device, sizeof(test_device), 0644);
 MODULE_PARM_DESC(device, "Bus ID of the DMA Engine to test (default: any)");
 
 static unsigned int threads_per_chan = 1;
-module_param(threads_per_chan, uint, S_IRUGO | S_IWUSR);
+module_param(threads_per_chan, uint, 0644);
 MODULE_PARM_DESC(threads_per_chan,
 		"Number of threads to start per channel (default: 1)");
 
 static unsigned int max_channels;
-module_param(max_channels, uint, S_IRUGO | S_IWUSR);
+module_param(max_channels, uint, 0644);
 MODULE_PARM_DESC(max_channels,
 		"Maximum number of channels to use (default: all)");
 
 static unsigned int iterations;
-module_param(iterations, uint, S_IRUGO | S_IWUSR);
+module_param(iterations, uint, 0644);
 MODULE_PARM_DESC(iterations,
 		"Iterations before stopping test (default: infinite)");
 
 static unsigned int dmatest;
-module_param(dmatest, uint, S_IRUGO | S_IWUSR);
+module_param(dmatest, uint, 0644);
 MODULE_PARM_DESC(dmatest,
 		"dmatest 0-memcpy 1-memset (default: 0)");
 
 static unsigned int xor_sources = 3;
-module_param(xor_sources, uint, S_IRUGO | S_IWUSR);
+module_param(xor_sources, uint, 0644);
 MODULE_PARM_DESC(xor_sources,
 		"Number of xor source buffers (default: 3)");
 
 static unsigned int pq_sources = 3;
-module_param(pq_sources, uint, S_IRUGO | S_IWUSR);
+module_param(pq_sources, uint, 0644);
 MODULE_PARM_DESC(pq_sources,
 		"Number of p+q source buffers (default: 3)");
 
 static int timeout = 3000;
-module_param(timeout, uint, S_IRUGO | S_IWUSR);
+module_param(timeout, uint, 0644);
 MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), "
 		 "Pass -1 for infinite timeout");
 
 static bool noverify;
-module_param(noverify, bool, S_IRUGO | S_IWUSR);
+module_param(noverify, bool, 0644);
 MODULE_PARM_DESC(noverify, "Disable random data setup and verification");
 
 static bool verbose;
-module_param(verbose, bool, S_IRUGO | S_IWUSR);
+module_param(verbose, bool, 0644);
 MODULE_PARM_DESC(verbose, "Enable \"success\" result messages (default: off)");
 
 /**
@@ -131,7 +129,7 @@ struct dmatest_params {
 	.get = dmatest_run_get,
 };
 static bool dmatest_run;
-module_param_cb(run, &run_ops, &dmatest_run, S_IRUGO | S_IWUSR);
+module_param_cb(run, &run_ops, &dmatest_run, 0644);
 MODULE_PARM_DESC(run, "Run the test (default: false)");
 
 /* Maximum amount of mismatched bytes in buffer to print */
@@ -216,7 +214,7 @@ static int dmatest_wait_get(char *val, const struct kernel_param *kp)
 	.get = dmatest_wait_get,
 	.set = param_set_bool,
 };
-module_param_cb(wait, &wait_ops, &wait, S_IRUGO);
+module_param_cb(wait, &wait_ops, &wait, 0444);
 MODULE_PARM_DESC(wait, "Wait for tests to complete (default: false)");
 
 static bool dmatest_match_channel(struct dmatest_params *params,