Re: [RFC] Fix typos

Previous thread: bdi-default hung waiting for kthread_stop to finish by Jeff Layton on Thursday, September 2, 2010 - 11:55 am. (1 message)

Next thread: [PATCH 1/3] perf, x86: Fix accidentally ack'ing a second event on intel perf counter by Don Zickus on Thursday, September 2, 2010 - 12:07 pm. (13 messages)
From: Lucas De Marchi
Date: Thursday, September 2, 2010 - 12:05 pm

Hi,

I'd like to know if it's worth fixing all typos found in comments,
documentation and sometimes code. A first attempt to do that you can
find at http://people.profusion.mobi/~lucas/kernel-typos-fix.patch

This was done by using the list of common typos available at
http://en.wikipedia.org/wiki/Wikipedia:Lists_of_common_misspellings

I already removed some false positives, but there are more. Before
going ahead and verifying all of them, I'd like to know whether this
would be a good patch.



-- 

Lucas De Marchi
--

From: Randy Dunlap
Date: Thursday, September 2, 2010 - 12:18 pm

Hi,

I'd take most of the corrections in Documentation/, but in general,
I don't think that it's a good idea without some careful review.

E.g.:

1.  changing thru to through isn't needed IMO.
Can you add "thru" to your dictionary?

2.
-(Warning, pulses on ACK ar inverted by transistor, irq is rised up on sync
+(Warning, pulses on ACK ar inverted by transistor, irq is rose up on sync

I'm not so sure about "is rose up" being better.  and what about "ar"?

I saw a few others that I could comment on, but I lost them in the 1.8 MB patch file.


diffstat summary for those interested:

 CREDITS                                                    |    6 -
 Documentation/ABI/testing/sysfs-bus-css                    |    2 
 Documentation/ABI/testing/sysfs-class                      |    2 
 Documentation/DocBook/dvb/dvbproperty.xml                  |    2 
 Documentation/DocBook/dvb/frontend.xml                     |    2 
 Documentation/DocBook/kernel-hacking.tmpl                  |    4 
 Documentation/DocBook/kernel-locking.tmpl                  |    2 
 Documentation/DocBook/libata.tmpl                          |    6 -
 Documentation/DocBook/mtdnand.tmpl                         |    6 -
 Documentation/DocBook/regulator.tmpl                       |    4 
 Documentation/DocBook/uio-howto.tmpl                       |    2 
 Documentation/DocBook/usb.tmpl                             |    2 
 Documentation/DocBook/v4l/common.xml                       |    2 
 Documentation/DocBook/v4l/controls.xml                     |    2 
 Documentation/DocBook/v4l/libv4l.xml                       |    2 
 Documentation/PCI/MSI-HOWTO.txt                            |    4 
 Documentation/RCU/trace.txt                                |    2 
 Documentation/SecurityBugs                                 |    2 
 Documentation/SubmittingDrivers                            |    2 
 Documentation/SubmittingPatches                            |    2 
 Documentation/arm/IXP4xx                          ...
From: Lucas De Marchi
Date: Thursday, September 2, 2010 - 12:31 pm

I plan to review it if it's indeed worth fixing those typos. I already
fixed several false positives like  "rela" being changed to "real",
but since it's a 1.8 MB patch I think it's better to ask first before

I'm only changing words with 3 chars or more, because otherwise I
would have a lot of false positives, but I think I can add this word

If you say it's good, I can review it more carefully.



-- 

Lucas De Marchi
--

From: Randy Dunlap
Date: Thursday, September 2, 2010 - 1:00 pm

It looks good for Documentation/.

And fixing typos is certainly something that we want to do in
user-visible messages, like printk strings.


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--

From: Lucas De Marchi
Date: Thursday, September 2, 2010 - 1:08 pm

What about comments and code?


-- 

Lucas De Marchi
--

From: Randy Dunlap
Date: Thursday, September 2, 2010 - 1:13 pm

That will require a lot more detailed review IMO.
Usually it is good to have, but not nearly as important as user-visible strings.

And you should copy trivial@kernel.org on the patch.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--

From: Lucas De Marchi
Date: Thursday, September 2, 2010 - 1:47 pm

Ok, I'll do in the v2 of this patch


-- 

Lucas De Marchi
--

From: Mike Frysinger
Date: Thursday, September 2, 2010 - 1:16 pm

i think it's useful to fix typos in those things, but this would def
require a human to review things.  it might also be saner to start
with a whitelist of common typos (doesnt checkpatch have this ?)
rather than doing it against a dictionary.

for example, we've committed a few drivers that used "platfrom" in
their structure names when it should have been "platform".
-mike
--

From: Lucas De Marchi
Date: Thursday, September 2, 2010 - 1:23 pm

I'm running a script with a dictionary of common typos, not a "normal

I didn't have this typo, but I can add it as well.


-- 

Lucas De Marchi
--

From: Randy Dunlap
Date: Thursday, September 2, 2010 - 1:25 pm

Kconfig prompts and help text are also user-visible strings.

I just saw this one:

bool "ET&T USB series TC4UM/TC5UH touchscreen controler support" if EMBEDDED

in drivers/input/touchscreen/Kconfig.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--

From: Lucas De Marchi
Date: Thursday, September 2, 2010 - 1:45 pm

I've just added the controler->controller fix to my dictionary.

-- 

Lucas De Marchi
--

From: Phillip Lougher
Date: Thursday, September 2, 2010 - 7:29 pm

On Thu, Sep 2, 2010 at 8:31 PM, Lucas De Marchi

The problem with "ar" is it is too short to know for certain what it
should be...  "ar" though likely a typo for "are" could also be a typo
for e.g. "arm", "arc", "arg", "ark", and "arp".

Perhaps in this and similar cases the script should flag these errors
for human attention rather than automatically fixing them.

Phillip
--

From: Lucas De Marchi
Date: Thursday, September 2, 2010 - 7:45 pm

Yes, after enabling it I've saw a lot of false positives and wrong
fixes. Even cases where "ar" was the correct one, referring to the ar
tool.

Now, I put this together with other warnings.



-- 

Lucas De Marchi
--

From: Lucas De Marchi
Date: Thursday, September 2, 2010 - 10:20 pm

Here is the second version. I've split the patch to be easier to
review. The first one is the patch generated in an automated way. The
second is the manual application of some fixes that were given as
warns when running the script. The third one is the removal of false
positives on Documentation/ as detailed by Phillip Lougher. Finally,
the fourth is all of them together.

http://people.profusion.mobi/~/lucas/kernel/0001-Automatic-spell-fixes.patch
http://people.profusion.mobi/~/lucas/kernel/0002-Manual-fixes-using-suggestions.patch
http://people.profusion.mobi/~/lucas/kernel/0003-Remove-false-positives-on-Documentati...
http://people.profusion.mobi/~/lucas/kernel/kernel-typos-fix-v2.patch

Below is the list of changes that were effectively applied on the first patch:


absense->absence
absolutly->absolutely
absorbtion->absorption
accelleration->acceleration
acceptible->acceptable
accessable->accessible
accidently->accidentally
accomodate->accommodate
accomodated->accommodated
accomodates->accommodates
accoring->according
accross->across
achievment->achievement
acommodate->accommodate
acomplish->accomplish
acomplished->accomplished
acording->according
activly->actively
additionaly->additionally
additonal->additional
addres->address
addresing->addressing
addressess->addresses
addtion->addition
addtional->additional
adminstered->administered
adminstrator->administrator
adn->and
adress->address
advertisment->advertisement
advertisments->advertisements
agains->against
aggresive->aggressive
aginst->against
agregates->aggregates
agressive->aggressive
agressively->aggressively
alledgedly->allegedly
allign->align
alligned->aligned
alltime->all-time
alot->a ...
From: Mike Frysinger
Date: Thursday, September 2, 2010 - 12:32 pm

"irq is raised on sync"
-mike
--

From: Phillip Lougher
Date: Thursday, September 2, 2010 - 7:17 pm

Seconded, even the files in Documentation/ (while mostly English text)
mix code and other technical/non-English details which throw up false
positives.

I reviewed the changes to the Documentation/ files and found the
following false positives

Documentation/DocBook/kernel-hacking.tmpl

- * Sun people can't spell worth damn. "compatability" indeed.
+ * Sun people can't spell worth damn. "compatibility" indeed.

-        .asciz "compatability"
+        .asciz "compatibility"

These misspellings are deliberate, and are in context to the file.

Documentation/hwmon/via686a

-        Jonathan Teh Soon Yew <j.teh@iname.com>
+        Jonathan Teh Soon Yew <j.the@iname.com>

teh is a name

Documentation/filesystems/vfat.txt

                 <slot #3, id = 0x43, characters = "h is long">
-                <slot #2, id = 0x02, characters = "xtension whic">
+                <slot #2, id = 0x02, characters = "xtension which">
                 <slot #1, id = 0x01, characters = "My Big File.E">

"xtension.whic" is correct, it is part of the string "My Big
File.Extension which is long" which has been split, the missing 'h' is
on the previous line.

Documentation/vm/pagemap.txt

-the read on an 8-byte boundary (e.g., if you seeked an odd number of bytes
+the read on an 8-byte boundary (e.g., if you sought an odd number of bytes

Thought seeked is bad English it is correct in this context where
you've done a seek operation.

The following are a couple of correctly found mistakes, but where
there's problems with the fix.

Documentation/filesystems/ntfs.txt

-to the "Target Device" or if you specified multipled target devices to all of
+to the "Target Device" or if you specified multiplied target devices to all of

multipled is a typo for multiple, not multiplied.

Documentation/scsi/aha152x.txt

-namely the address space is limited to upto 255 heads, upto 63 sectors
+namely the address space is limited to up to 255 heads, upto 63 sectors

What happened here?  It fixed ...
From: Paul E. McKenney
Date: Tuesday, September 7, 2010 - 9:06 am

I am messing with Documentation/RCU/trace.txt anyway, so I took the
s/thoughout/throughout/, thank you!!!

--

From: Len Brown
Date: Tuesday, September 7, 2010 - 9:47 am

The fixes to the ACPI and idle-related files below are correct
and so I'll apply them now - thanks.

drivers/acpi/acpica/exutils.c
drivers/acpi/acpica/rsutils.c
drivers/acpi/apei/Kconfig
drivers/acpi/apei/erst-dbg.c
drivers/acpi/apei/erst.c
drivers/acpi/bus.c
drivers/acpi/processor_perflib.c
include/acpi/acpixf.h

drivers/cpuidle/governors/menu.c

It may be problematic to apply a single jumbo patch to the entire kernel
b/c it will surely conflict with code that is changing.

thanks,
Len Brown, Intel Open Source Technology Center

--

From: Lucas De Marchi
Date: Tuesday, September 7, 2010 - 10:01 am

I think at least for Documentation it can be applied all at once. I
reviewed again all changes to this directory and updated the patches
to my git repository:
git://git.profusion.mobi/users/lucas/linux-2.6.git spell-fix



Lucas De Marchi
--

From: Jason Wessel
Date: Tuesday, September 7, 2010 - 10:30 am

I am in agreement with Randy on his point that any changes should get
reviewed if possible by the people who own the files or authored the
comments.

I reviewed all the pieces that were kdb / kgdb related which include
the following and all the changes are ok (one exception noted).

Documentation/kernel-parameters.txt
arch/cris/arch-v32/kernel/kgdb.c
drivers/misc/kgdbts.c
kernel/debug/kdb/kdb_main.c
kernel/debug/kdb/kdb_support.c

--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -277,7 +277,7 @@ static int hw_break_release_slot(int breakno)
 		pevent = per_cpu_ptr(breakinfo[breakno].pev, cpu);
 		if (dbg_release_bp_slot(*pevent))
 			/*
-			 * The debugger is responisble for handing the retry on
+			 * The debugger is responsible for handing the retry on

---- from Jason ----

This should actually be:

+			 * The debugger is responsible for handling the retry on

--------------------

Thanks,
Jason.
--

Previous thread: bdi-default hung waiting for kthread_stop to finish by Jeff Layton on Thursday, September 2, 2010 - 11:55 am. (1 message)

Next thread: [PATCH 1/3] perf, x86: Fix accidentally ack'ing a second event on intel perf counter by Don Zickus on Thursday, September 2, 2010 - 12:07 pm. (13 messages)