[PATCH V2 0/7] usb-misc: sisusbvga: cleanup and bug fix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux