* [PATCH 0967/1285] Replace numeric parameter like 0444 with macro @ 2016-08-02 12:03 Baole Ni 2016-08-02 13:19 ` Matwey V. Kornilov 0 siblings, 1 reply; 4+ messages in thread From: Baole Ni @ 2016-08-02 12:03 UTC (permalink / raw) To: gregkh, jslaby, m.chehab, m.szyprowski, kyungmin.park, k.kozlowski Cc: linux-serial, linux-kernel, andriy.shevchenko, phillip.raffeck, anton.wuerfel, peter, yamada.masahiro, wan.ahmad.zainie.wan.mohamad, mail, matwey, chuansheng.liu, baolex.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/tty/serial/8250/8250_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 0fbd7c0..9309ec1 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -1188,17 +1188,17 @@ module_exit(serial8250_exit); MODULE_LICENSE("GPL"); MODULE_DESCRIPTION("Generic 8250/16x50 serial driver"); -module_param(share_irqs, uint, 0644); +module_param(share_irqs, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); MODULE_PARM_DESC(share_irqs, "Share IRQs with other non-8250/16x50 devices (unsafe)"); -module_param(nr_uarts, uint, 0644); +module_param(nr_uarts, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); MODULE_PARM_DESC(nr_uarts, "Maximum number of UARTs supported. (1-" __MODULE_STRING(CONFIG_SERIAL_8250_NR_UARTS) ")"); -module_param(skip_txen_test, uint, 0644); +module_param(skip_txen_test, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); MODULE_PARM_DESC(skip_txen_test, "Skip checking for the TXEN bug at init time"); #ifdef CONFIG_SERIAL_8250_RSA -module_param_array(probe_rsa, ulong, &probe_rsa_count, 0444); +module_param_array(probe_rsa, ulong, &probe_rsa_count, S_IRUSR | S_IRGRP | S_IROTH); MODULE_PARM_DESC(probe_rsa, "Probe I/O ports for RSA"); #endif MODULE_ALIAS_CHARDEV_MAJOR(TTY_MAJOR); -- 2.9.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 0967/1285] Replace numeric parameter like 0444 with macro 2016-08-02 12:03 [PATCH 0967/1285] Replace numeric parameter like 0444 with macro Baole Ni @ 2016-08-02 13:19 ` Matwey V. Kornilov 2016-08-02 13:34 ` Matwey V. Kornilov 0 siblings, 1 reply; 4+ messages in thread From: Matwey V. Kornilov @ 2016-08-02 13:19 UTC (permalink / raw) To: Baole Ni Cc: Greg KH, Jiri Slaby, m.chehab, m.szyprowski, kyungmin.park, k.kozlowski, linux-serial, linux-kernel, andriy.shevchenko, phillip.raffeck, anton.wuerfel, Peter Hurley, yamada.masahiro, wan.ahmad.zainie.wan.mohamad, mail, chuansheng.liu Hello, I believe that 0644 is shorter and easier to read and understand than the long list of macros like S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH where it is easier to miss something. 2016-08-02 15:03 GMT+03:00 Baole Ni <baolex.ni@intel.com>: > 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/tty/serial/8250/8250_core.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index 0fbd7c0..9309ec1 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -1188,17 +1188,17 @@ module_exit(serial8250_exit); > MODULE_LICENSE("GPL"); > MODULE_DESCRIPTION("Generic 8250/16x50 serial driver"); > > -module_param(share_irqs, uint, 0644); > +module_param(share_irqs, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > MODULE_PARM_DESC(share_irqs, "Share IRQs with other non-8250/16x50 devices (unsafe)"); > > -module_param(nr_uarts, uint, 0644); > +module_param(nr_uarts, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > MODULE_PARM_DESC(nr_uarts, "Maximum number of UARTs supported. (1-" __MODULE_STRING(CONFIG_SERIAL_8250_NR_UARTS) ")"); > > -module_param(skip_txen_test, uint, 0644); > +module_param(skip_txen_test, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > MODULE_PARM_DESC(skip_txen_test, "Skip checking for the TXEN bug at init time"); > > #ifdef CONFIG_SERIAL_8250_RSA > -module_param_array(probe_rsa, ulong, &probe_rsa_count, 0444); > +module_param_array(probe_rsa, ulong, &probe_rsa_count, S_IRUSR | S_IRGRP | S_IROTH); > MODULE_PARM_DESC(probe_rsa, "Probe I/O ports for RSA"); > #endif > MODULE_ALIAS_CHARDEV_MAJOR(TTY_MAJOR); > -- > 2.9.2 > -- With best regards, Matwey V. Kornilov. Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 0967/1285] Replace numeric parameter like 0444 with macro 2016-08-02 13:19 ` Matwey V. Kornilov @ 2016-08-02 13:34 ` Matwey V. Kornilov 2016-08-02 13:46 ` Greg KH 0 siblings, 1 reply; 4+ messages in thread From: Matwey V. Kornilov @ 2016-08-02 13:34 UTC (permalink / raw) To: Baole Ni Cc: Greg KH, Jiri Slaby, m.chehab, m.szyprowski, kyungmin.park, k.kozlowski, linux-serial, linux-kernel, andriy.shevchenko, phillip.raffeck, anton.wuerfel, Peter Hurley, yamada.masahiro, wan.ahmad.zainie.wan.mohamad, mail, chuansheng.liu And actually S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH can be simplified to S_IRUGO | S_IWUSR, not? I think we need to have and use dedicated special macros for most common magic octal permissions like 0755 or 0644. 2016-08-02 16:19 GMT+03:00 Matwey V. Kornilov <matwey@sai.msu.ru>: > Hello, > > I believe that 0644 is shorter and easier to read and understand than > the long list of macros like S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH > where it is easier to miss something. > > 2016-08-02 15:03 GMT+03:00 Baole Ni <baolex.ni@intel.com>: >> 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/tty/serial/8250/8250_core.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c >> index 0fbd7c0..9309ec1 100644 >> --- a/drivers/tty/serial/8250/8250_core.c >> +++ b/drivers/tty/serial/8250/8250_core.c >> @@ -1188,17 +1188,17 @@ module_exit(serial8250_exit); >> MODULE_LICENSE("GPL"); >> MODULE_DESCRIPTION("Generic 8250/16x50 serial driver"); >> >> -module_param(share_irqs, uint, 0644); >> +module_param(share_irqs, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); >> MODULE_PARM_DESC(share_irqs, "Share IRQs with other non-8250/16x50 devices (unsafe)"); >> >> -module_param(nr_uarts, uint, 0644); >> +module_param(nr_uarts, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); >> MODULE_PARM_DESC(nr_uarts, "Maximum number of UARTs supported. (1-" __MODULE_STRING(CONFIG_SERIAL_8250_NR_UARTS) ")"); >> >> -module_param(skip_txen_test, uint, 0644); >> +module_param(skip_txen_test, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); >> MODULE_PARM_DESC(skip_txen_test, "Skip checking for the TXEN bug at init time"); >> >> #ifdef CONFIG_SERIAL_8250_RSA >> -module_param_array(probe_rsa, ulong, &probe_rsa_count, 0444); >> +module_param_array(probe_rsa, ulong, &probe_rsa_count, S_IRUSR | S_IRGRP | S_IROTH); >> MODULE_PARM_DESC(probe_rsa, "Probe I/O ports for RSA"); >> #endif >> MODULE_ALIAS_CHARDEV_MAJOR(TTY_MAJOR); >> -- >> 2.9.2 >> > > > > -- > With best regards, > Matwey V. Kornilov. > Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia > 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382 -- With best regards, Matwey V. Kornilov. Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 0967/1285] Replace numeric parameter like 0444 with macro 2016-08-02 13:34 ` Matwey V. Kornilov @ 2016-08-02 13:46 ` Greg KH 0 siblings, 0 replies; 4+ messages in thread From: Greg KH @ 2016-08-02 13:46 UTC (permalink / raw) To: Matwey V. Kornilov Cc: Baole Ni, Jiri Slaby, m.chehab, m.szyprowski, kyungmin.park, k.kozlowski, linux-serial, linux-kernel, andriy.shevchenko, phillip.raffeck, anton.wuerfel, Peter Hurley, yamada.masahiro, wan.ahmad.zainie.wan.mohamad, mail, chuansheng.liu On Tue, Aug 02, 2016 at 04:34:20PM +0300, Matwey V. Kornilov wrote: > And actually S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH can be simplified > to S_IRUGO | S_IWUSR, not? > I think we need to have and use dedicated special macros for most > common magic octal permissions like 0755 or 0644. We already do have them, the developer here decided to not use them for some unknown reason, making all 1285 of these patches wrong :( ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-02 16:58 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-08-02 12:03 [PATCH 0967/1285] Replace numeric parameter like 0444 with macro Baole Ni 2016-08-02 13:19 ` Matwey V. Kornilov 2016-08-02 13:34 ` Matwey V. Kornilov 2016-08-02 13:46 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox