Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1310325
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH V2 0/7] usb-misc: sisusbvga: cleanup and bug fix |
| Date | 2016-01-15 18:50 +0100 |
| Message-ID | <qRgFz-W7-11@gated-at.bofh.it> (permalink) |
| Organization | linux.* mail to news gateway |
The file drivers/usb/misc/sisusbvga/sisusb.c had many (192) coding style issues
reported by checkpatch. This file also had a problematic error path in the
probe function that could result in dereferencing a null pointer.
This patch series fix coding style issues and a problematic error path which
could result in a null pointer dereference.
Patch 1 and 2 change whitespace only, patch 3 to 6 fix various coding style
issues, and patch 7 fix a null pointer dereference bug.
Joe Perches suggested me to include objtdiff output for patches that are not
supposed to make semantic changes, but it is not working well for me with gcc
(GCC) 5.3.1 20151207 (Red Hat 5.3.1-2). Even compiling the same source code
produces different output from objdump. The objdump command I'm using is from
./scripts/objdiff. See an example:
# A patch that should not make any semantic change
$ cat /tmp/patch
diff --git a/drivers/usb/misc/sisusbvga/sisusb.c b/drivers/usb/misc/sisusbvga/sisusb.c
index 8efbaba..a48b086d 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -1353,7 +1353,7 @@ sisusb_testreadwrite(struct sisusb_usb_data *sisusb)
static char srcbuffer[] = { 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77 };
char destbuffer[10];
size_t dummy;
- int i,j;
+ int i, j;
sisusb_copy_memory(sisusb, srcbuffer, sisusb->vrambase, 7, &dummy);
# Lets compile and collect a dump based on linux-next/master
$ git checkout linux-next/master
$ md5sum drivers/usb/misc/sisusbvga/sisusb.c
d6ffbd44f3f1cf81fd55ce84441ab889 drivers/usb/misc/sisusbvga/sisusb.c
$ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
$ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/base_dump
# Now let's apply the patch and collect other dump
$ git apply /tmp/patch
$ md5sum drivers/usb/misc/sisusbvga/sisusb.c
0b7d579c8ae2159f677c6a5c6efc4956 drivers/usb/misc/sisusbvga/sisusb.c
$ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
$ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/dump
# I was expecting the diff to be empty
$ diff /tmp/base_dump /tmp/dump
9135,9136c9135
< : 8e 4d 31 mov 0x31(%rbp),%cs
< : 46 00 00 rex.RX add %r8b,(%rax)
---
> : 25 c4 31 46 00 and $0x4631c4,%eax
9139c9138
< : 4b 00 00 rex.WXB add %al,(%r8)
---
> : 00 4b 00 add %cl,0x0(%rbx)
# But here is the interesting part. Even compiling the exact same source code
# produces different results
$ git checkout -- .
$ md5sum drivers/usb/misc/sisusbvga/sisusb.c
d6ffbd44f3f1cf81fd55ce84441ab889 drivers/usb/misc/sisusbvga/sisusb.c
$ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
$ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/base_dump_again
$ diff /tmp/base_dump /tmp/base_dump_again
9135,9136c9135,9136
< : 8e 4d 31 mov 0x31(%rbp),%cs
< : 46 00 00 rex.RX add %r8b,(%rax)
---
> : de 10 ficom (%rax)
> : 33 46 00 xor 0x0(%rsi),%eax
9139c9139
< : 4b 00 00 rex.WXB add %al,(%r8)
---
> : 00 4b 00 add %cl,0x0(%rbx)
Peter Senna Tschudin (7):
usb-misc: sisusbvga: Fix coding style: horizontal whitespace changes
usb-misc: sisusbvga: Fix coding style: vertical whitespace changes
usb-misc: sisusbvga: Fix coding style: braces, parenthesis, comment
usb-misc: sisusbvga: Fix coding style: remove assignment from if tests
usb-misc: sisusbvga: Remove null test before calls to kfree()
usb-misc: sisusbvga: Remove memory allocation logs
usb-misc: sisusbvga: fix error path
drivers/usb/misc/sisusbvga/sisusb.c | 1543 +++++++++++++++++------------------
1 file changed, 752 insertions(+), 791 deletions(-)
--
2.5.0
Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread
[PATCH V2 0/7] usb-misc: sisusbvga: cleanup and bug fix Peter Senna Tschudin <peter.senna@collabora.com> - 2016-01-15 18:50 +0100
[PATCH V2 5/7] usb-misc: sisusbvga: Remove null test before calls to kfree() Peter Senna Tschudin <peter.senna@collabora.com> - 2016-01-15 18:50 +0100
Re: [PATCH V2 0/7] usb-misc: sisusbvga: cleanup and bug fix Joe Perches <joe@perches.com> - 2016-01-15 19:00 +0100
Re: [PATCH V2 0/7] usb-misc: sisusbvga: cleanup and bug fix Jason Cooper <jason@lakedaemon.net> - 2016-01-15 19:10 +0100
Re: [PATCH V2 1/7] usb-misc: sisusbvga: Fix coding style: horizontal whitespace changes Joe Perches <joe@perches.com> - 2016-01-15 19:00 +0100
Re: [PATCH V2 1/7] usb-misc: sisusbvga: Fix coding style: horizontal whitespace changes "Peter Senna" <peter.senna@collabora.co.uk> - 2016-01-15 19:10 +0100
Re: Re: [PATCH V2 1/7] usb-misc: sisusbvga: Fix coding style: horizontal whitespace changes Alan Stern <stern@rowland.harvard.edu> - 2016-01-15 19:30 +0100
csiph-web