Message-ID: <CAE8KmOxG=3sWKpeB5fdWTK-SCipS=JyDE-_DNgY--DtoSQZ0Qw@mail.gmail.com>
Date: Tue, 26 Mar 2024 15:15:00 +0530
From: Prasad Pandit <ppandit@...hat.com>
To: Dan Carpenter <dan.carpenter@...aro.org>, Greg KH <gregkh@...uxfoundation.org>
Cc: florian.fainelli@...adcom.com, bcm-kernel-feedback-list@...adcom.com, 
	linux-arm-kernel@...ts.infradead.org, rjui@...adcom.com, 
	sbranden@...adcom.com, linux-staging@...ts.linux.dev, 
	linux-rpi-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	Prasad Pandit <pjp@...oraproject.org>
Subject: Re: [PATCH v3] staging: bcm2835-audio: add terminating new line to Kconfig

Hello Greg, Dan

On Tue, 26 Mar 2024 at 01:12, Dan Carpenter <dan.carpenter@...aro.org> wrote:
> On Mon, Mar 25, 2024 at 07:04:15PM +0100, Greg KH wrote:
>>  If it's an out-of-tree parser, that's different, and the tool should be fixed, no
>> need to touch the kernel files for no good reason.
> It's annoying to cat a file when it doesn't have a newline on the end...
>
> dcarpenter@...oto:~/progs/kernel/trees$ cat -n drivers/staging/vc04_services/bcm2835-audio/Kconfig
>     11            running on VideoCore.dcarpenter@...oto:~/progs/kernel/trees$
>                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> So you could resend with that as a justification.  But, yeah, it's a
> good idea to fix the tool as well.

* I'm trying to fix errors reported by the config-kernel[1] tool. It
is an out-of-tree parser.

* IMHO, this patch should be evaluated on whether not having the
terminating new-line character is right or wrong? Saying that patch is
not required because we don't know if it breaks an in-tree parser OR
that patch is acceptable for the annoyance while using cat(1) command
does not seem right.

* As for fixing the parser, I did try to do that in the past but did
not find a good fix for it, I'll try again. Meanwhile, I tried to find
out how many such files are there?
===
$ tf=0 df=0; for f in $(find . -path "./[a-zA-Z0-9]*" -name Kconfig*);
do tf=$(($tf+1)); eof=$(tail -c1 $f); if [ -n "$eof" -a "$eof" != "\n"
]; then echo $f; df=$(($df+1)); fi done 2> /dev/null; echo
"No-NL-Files/Total-Files: $df/$tf";
/drivers/staging/vc04_services/bcm2835-audio/Kconfig
/drivers/media/dvb-frontends/cxd2880/Kconfig
No-NL-Files/Total-Files: 2/1698

$ tf=0 df=0; for f in $(find . -path "./[a-zA-Z0-9]*" -type f); do
tf=$(($tf+1)); eof=$(tail -c1 $f); if [ -n "$eof" -a "$eof" != "\n" ];
then echo $f; df=$(($df+1)); fi done 2> /dev/null; echo
"No-NL-Files/Total-Files: $df/$tf";
/drivers/gpu/drm/i915/gt/shaders/README
/drivers/gpu/drm/xe/display/xe_fb_pin.c
/drivers/gpu/drm/sprd/Makefile
..
No-NL-Files/Total-Files: 65/84319
===

* There are 2 Kconfig files which don't terminate with a new-line
character. I have sent fix patches for both of them. Rest of all
~99.90+% regular files follow the norm of terminating with a new-line
character. Clearly files not terminating with a new-line are erroneous
and should be fixed. Suggesting to fix the parsers to handle erroneous
input files, which can be easily fixed in the source tree itself, is
not reasonable. Maybe as Dan suggested, it'll help to add a check to
'checkpatch.pl' script to flag patches which try to add files without
terminating new-lines. I tried to look into checkpatch.pl script, but
it's quite big to understand in a day or two.

* I'm okay to resend the patch saying annoyance while using cat(1)
command as justification if that works. If in-tree parser is the only
criteria, what would be the process to make config-kernel[1] tool an
in-tree parser? I can try to follow that process.

Thank you.
---
  - Prasad
[1] https://github.com/pjps/config-kernel

Powered by blists - more mailing lists