* [PATCH] locks: remove trailing semicolon in macro definition
@ 2020-11-27 19:07 trix
2020-11-27 19:53 ` Matthew Wilcox
0 siblings, 1 reply; 6+ messages in thread
From: trix @ 2020-11-27 19:07 UTC (permalink / raw)
To: jlayton, bfields, viro; +Cc: linux-fsdevel, linux-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
The macro use will already have a semicolon.
Signed-off-by: Tom Rix <trix@redhat.com>
---
fs/fcntl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 05b36b28f2e8..96a65758c498 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -526,7 +526,7 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
(dst)->l_whence = (src)->l_whence; \
(dst)->l_start = (src)->l_start; \
(dst)->l_len = (src)->l_len; \
- (dst)->l_pid = (src)->l_pid;
+ (dst)->l_pid = (src)->l_pid
static int get_compat_flock(struct flock *kfl, const struct compat_flock __user *ufl)
{
--
2.18.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] locks: remove trailing semicolon in macro definition
2020-11-27 19:07 [PATCH] locks: remove trailing semicolon in macro definition trix
@ 2020-11-27 19:53 ` Matthew Wilcox
2020-11-29 17:47 ` Tom Rix
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Wilcox @ 2020-11-27 19:53 UTC (permalink / raw)
To: trix; +Cc: jlayton, bfields, viro, linux-fsdevel, linux-kernel
On Fri, Nov 27, 2020 at 11:07:07AM -0800, trix@redhat.com wrote:
> +++ b/fs/fcntl.c
> @@ -526,7 +526,7 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
> (dst)->l_whence = (src)->l_whence; \
> (dst)->l_start = (src)->l_start; \
> (dst)->l_len = (src)->l_len; \
> - (dst)->l_pid = (src)->l_pid;
> + (dst)->l_pid = (src)->l_pid
This should be wrapped in a do { } while (0).
Look, this warning is clearly great at finding smelly code, but the
fixes being generated to shut up the warning are low quality.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] locks: remove trailing semicolon in macro definition
2020-11-27 19:53 ` Matthew Wilcox
@ 2020-11-29 17:47 ` Tom Rix
2020-11-29 17:52 ` Randy Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Tom Rix @ 2020-11-29 17:47 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: jlayton, bfields, viro, linux-fsdevel, linux-kernel
On 11/27/20 11:53 AM, Matthew Wilcox wrote:
> On Fri, Nov 27, 2020 at 11:07:07AM -0800, trix@redhat.com wrote:
>> +++ b/fs/fcntl.c
>> @@ -526,7 +526,7 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
>> (dst)->l_whence = (src)->l_whence; \
>> (dst)->l_start = (src)->l_start; \
>> (dst)->l_len = (src)->l_len; \
>> - (dst)->l_pid = (src)->l_pid;
>> + (dst)->l_pid = (src)->l_pid
> This should be wrapped in a do { } while (0).
>
> Look, this warning is clearly great at finding smelly code, but the
> fixes being generated to shut up the warning are low quality.
>
Multiline macros not following the do {} while (0) pattern are likely a larger problem.
I'll take a look.
Thanks,
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] locks: remove trailing semicolon in macro definition
2020-11-29 17:47 ` Tom Rix
@ 2020-11-29 17:52 ` Randy Dunlap
2020-11-29 18:15 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2020-11-29 17:52 UTC (permalink / raw)
To: Tom Rix, Matthew Wilcox
Cc: jlayton, bfields, viro, linux-fsdevel, linux-kernel
On 11/29/20 9:47 AM, Tom Rix wrote:
>
> On 11/27/20 11:53 AM, Matthew Wilcox wrote:
>> On Fri, Nov 27, 2020 at 11:07:07AM -0800, trix@redhat.com wrote:
>>> +++ b/fs/fcntl.c
>>> @@ -526,7 +526,7 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
>>> (dst)->l_whence = (src)->l_whence; \
>>> (dst)->l_start = (src)->l_start; \
>>> (dst)->l_len = (src)->l_len; \
>>> - (dst)->l_pid = (src)->l_pid;
>>> + (dst)->l_pid = (src)->l_pid
>> This should be wrapped in a do { } while (0).
>>
>> Look, this warning is clearly great at finding smelly code, but the
>> fixes being generated to shut up the warning are low quality.
>>
> Multiline macros not following the do {} while (0) pattern are likely a larger problem.
>
> I'll take a look.
Could it become a static inline function instead?
or that might expand its scope too much?
--
~Randy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] locks: remove trailing semicolon in macro definition
2020-11-29 17:52 ` Randy Dunlap
@ 2020-11-29 18:15 ` James Bottomley
2020-11-29 22:31 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2020-11-29 18:15 UTC (permalink / raw)
To: Randy Dunlap, Tom Rix, Matthew Wilcox
Cc: jlayton, bfields, viro, linux-fsdevel, linux-kernel
On Sun, 2020-11-29 at 09:52 -0800, Randy Dunlap wrote:
> On 11/29/20 9:47 AM, Tom Rix wrote:
> > On 11/27/20 11:53 AM, Matthew Wilcox wrote:
> > > On Fri, Nov 27, 2020 at 11:07:07AM -0800, trix@redhat.com wrote:
> > > > +++ b/fs/fcntl.c
> > > > @@ -526,7 +526,7 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd,
> > > > unsigned int, cmd,
> > > > (dst)->l_whence = (src)->l_whence; \
> > > > (dst)->l_start = (src)->l_start; \
> > > > (dst)->l_len = (src)->l_len; \
> > > > - (dst)->l_pid = (src)->l_pid;
> > > > + (dst)->l_pid = (src)->l_pid
> > > This should be wrapped in a do { } while (0).
> > >
> > > Look, this warning is clearly great at finding smelly code, but
> > > the
> > > fixes being generated to shut up the warning are low quality.
> > >
> > Multiline macros not following the do {} while (0) pattern are
> > likely a larger problem.
> >
> > I'll take a look.
>
> Could it become a static inline function instead?
> or that might expand its scope too much?
I think nowadays we should always use static inlines for argument
checking unless we're capturing debug information like __FILE__ or
__LINE__ or something that a static inline can't. Even in the latter
case the pattern should probably be single line #define that captures
the information and passes it to a static inline.
There was a time when we had problems with compiler expansion of static
inlines, so we shouldn't go back and churn the code base to change it
because there's thousands of these and possibly some old compiler used
for an obscure architecture that still needs the define.
James
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] locks: remove trailing semicolon in macro definition
2020-11-29 18:15 ` James Bottomley
@ 2020-11-29 22:31 ` Joe Perches
0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2020-11-29 22:31 UTC (permalink / raw)
To: James Bottomley, Randy Dunlap, Tom Rix, Matthew Wilcox
Cc: jlayton, bfields, viro, linux-fsdevel, linux-kernel
On Sun, 2020-11-29 at 10:15 -0800, James Bottomley wrote:
> I think nowadays we should always use static inlines for argument
> checking unless we're capturing debug information like __FILE__ or
> __LINE__ or something that a static inline can't.
IMO: __LINE__ should never be used.
__func__ is the only thing that can't be captured correctly as
the inline gets its own name.
__builtin_return_address(1) would generally work well enough
for the inlines.
> There was a time when we had problems with compiler expansion of static
> inlines, so we shouldn't go back and churn the code base to change it
> because there's thousands of these and possibly some old compiler used
> for an obscure architecture that still needs the define.
That's not a very compelling argument to me.
Those old compilers and obscure architectures should continue
to use the old versions of the code.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-29 22:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-27 19:07 [PATCH] locks: remove trailing semicolon in macro definition trix
2020-11-27 19:53 ` Matthew Wilcox
2020-11-29 17:47 ` Tom Rix
2020-11-29 17:52 ` Randy Dunlap
2020-11-29 18:15 ` James Bottomley
2020-11-29 22:31 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).