Messages in this thread | | | Subject | Re: [PATCH] checkpatch: remove unneeded or ("|") | From | Joe Perches <> | Date | Tue, 24 Mar 2015 07:11:57 -0700 |
| |
On Tue, 2015-03-24 at 12:54 +0100, Christian Borntraeger wrote: > Am 12.03.2015 um 15:56 schrieb Andy Whitcroft: > > On Thu, Mar 12, 2015 at 07:13:35AM -0700, Joe Perches wrote: > >> On Thu, 2015-03-12 at 15:07 +0100, Christian Borntraeger wrote: > >>> while porting commit 89a883530fe7 ("checkpatch: ## is not a valid > >>> modifier") to QEMU, Peter Maydell noticed that the | at the end of > >>> the list is not necessary. > >>> > >>> Lets get rid of it in kernel checkpatch.pl > >> > >> Andy? Was that meant to allow nothing? > >> > >>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > >> [] > >>> @@ -1550,7 +1550,7 @@ sub possible { > >>> asm|__asm__| > >>> do| > >>> \#| > >>> - \#\#| > >>> + \#\# > >>> )(?:\s|$)| > >>> ^(?:typedef|struct|enum)\b > >>> )}x; > > > > Blimey you tax my memory indeed. > > > > The two places it is used we are saying that the strings which match are > > not modifiers. So it seems sane that we would want to reject the empty > > string in that case. That said, it does not appear any of the callers > > would call with a blank string. I would suggest we had it like that as > > a safety feature, _though_ if we had I would have expected it to have a > > nice shiney comment to say just how smart we being using that trailing > > or and so likely as not is is unintentional. > > > > tl;dr I think it is safe to elide it with the current callers, it being > > there seems safe, but if it stays damn it should have a comment to say > > its a safety net and not just hide out. > > > > -apw > > > > So how to proceed? > Take my patch, drop my patch or rework my patch?
If Andy acks it, I suggest sending it again along with Andy's ack, to Andrew Morton (cc'd)
| |