| Message ID | 20170127192603.GA13522@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
On Fri, 2017-01-27 at 22:26 +0300, Pichugin Dmitry wrote: > This fixes the checkpatch.pl warnings: > * Macros should not use a trailing semicolon. > * Spaces required around that '='. > * Symbolic permissions 'S_IRUGO' are not preferred. OK > * Macro argument reuse 'buflen' - possible side-effects Not all checkpatch messages need fixing. This is one of them. > diff --git a/net/wireless/debugfs.c b/net/wireless/debugfs.c [] > @@ -17,11 +17,12 @@ > static ssize_t name## _read(struct file *file, char __user *userbuf, \ > size_t count, loff_t *ppos) \ > { \ > - struct wiphy *wiphy= file->private_data; \ > - char buf[buflen]; \ > + struct wiphy *wiphy = file->private_data; \ > + int __buflen = __builtin_constant_p(buflen) ? buflen : -1; \ > + char buf[__buflen]; \ That's rather an odd change too
On Fri, 2017-01-27 at 22:26 +0300, Pichugin Dmitry wrote: > This fixes the checkpatch.pl warnings: > * Macros should not use a trailing semicolon. > * Spaces required around that '='. > * Symbolic permissions 'S_IRUGO' are not preferred. > * Macro argument reuse 'buflen' - possible side-effects I really see no point in any of this. johannes
On Fri, 2017-01-27 at 22:00 +0100, Johannes Berg wrote: > On Fri, 2017-01-27 at 22:26 +0300, Pichugin Dmitry wrote: > > This fixes the checkpatch.pl warnings: > > * Macros should not use a trailing semicolon. > > * Spaces required around that '='. > > * Symbolic permissions 'S_IRUGO' are not preferred. > > * Macro argument reuse 'buflen' - possible side-effects > > I really see no point in any of this. Look at the uses of DEBUGFS_READONLY_FILE and see if they are consistent before and after. DEBUGFS_READONLY_FILE(rts_threshold, 20, "%d", - wiphy->rts_threshold) + wiphy->rts_threshold); DEBUGFS_READONLY_FILE(fragmentation_threshold, 20, "%d", wiphy->frag_threshold); DEBUGFS_READONLY_FILE(short_retry_limit, 20, "%d", - wiphy->retry_short) + wiphy->retry_short); DEBUGFS_READONLY_FILE(long_retry_limit, 20, "%d", wiphy->retry_long);
On Fri, Jan 27, 2017 at 11:48:35AM -0800, Joe Perches wrote: > On Fri, 2017-01-27 at 22:26 +0300, Pichugin Dmitry wrote: > > This fixes the checkpatch.pl warnings: > > * Macros should not use a trailing semicolon. > > * Spaces required around that '='. > > * Symbolic permissions 'S_IRUGO' are not preferred. > > OK > > > * Macro argument reuse 'buflen' - possible side-effects > > Not all checkpatch messages need fixing. > This is one of them. > > > diff --git a/net/wireless/debugfs.c b/net/wireless/debugfs.c > [] > > @@ -17,11 +17,12 @@ > > static ssize_t name## _read(struct file *file, char __user *userbuf, \ > > size_t count, loff_t *ppos) \ > > { \ > > - struct wiphy *wiphy= file->private_data; \ > > - char buf[buflen]; \ > > + struct wiphy *wiphy = file->private_data; \ > > + int __buflen = __builtin_constant_p(buflen) ? buflen : -1; \ > > + char buf[__buflen]; \ > > That's rather an odd change too > OK. I will update the patch. Best Regards, Dmitriy.
diff --git a/net/wireless/debugfs.c b/net/wireless/debugfs.c index 5d45391..96e3cff 100644 --- a/net/wireless/debugfs.c +++ b/net/wireless/debugfs.c @@ -17,11 +17,12 @@ static ssize_t name## _read(struct file *file, char __user *userbuf, \ size_t count, loff_t *ppos) \ { \ - struct wiphy *wiphy= file->private_data; \ - char buf[buflen]; \ + struct wiphy *wiphy = file->private_data; \ + int __buflen = __builtin_constant_p(buflen) ? buflen : -1; \ + char buf[__buflen]; \ int res; \ \ - res = scnprintf(buf, buflen, fmt "\n", ##value); \ + res = scnprintf(buf, __buflen, fmt "\n", ##value); \ return simple_read_from_buffer(userbuf, count, ppos, buf, res); \ } \ \ @@ -29,14 +30,14 @@ static const struct file_operations name## _ops = { \ .read = name## _read, \ .open = simple_open, \ .llseek = generic_file_llseek, \ -}; +} DEBUGFS_READONLY_FILE(rts_threshold, 20, "%d", - wiphy->rts_threshold) + wiphy->rts_threshold); DEBUGFS_READONLY_FILE(fragmentation_threshold, 20, "%d", wiphy->frag_threshold); DEBUGFS_READONLY_FILE(short_retry_limit, 20, "%d", - wiphy->retry_short) + wiphy->retry_short); DEBUGFS_READONLY_FILE(long_retry_limit, 20, "%d", wiphy->retry_long); @@ -103,7 +104,7 @@ static const struct file_operations ht40allow_map_ops = { }; #define DEBUGFS_ADD(name) \ - debugfs_create_file(#name, S_IRUGO, phyd, &rdev->wiphy, &name## _ops); + debugfs_create_file(#name, 0444, phyd, &rdev->wiphy, &name## _ops) void cfg80211_debugfs_rdev_add(struct cfg80211_registered_device *rdev) {
This fixes the checkpatch.pl warnings: * Macros should not use a trailing semicolon. * Spaces required around that '='. * Symbolic permissions 'S_IRUGO' are not preferred. * Macro argument reuse 'buflen' - possible side-effects Signed-off-by: Pichugin Dmitry <smokeman85@gmail.com> --- net/wireless/debugfs.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)