public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Senna Tschudin <peter.senna@collabora.com>
To: thomas@winischhofer.net, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	joe@perches.com, sergei.shtylyov@cogentembedded.com
Cc: Peter Senna Tschudin <peter.senna@collabora.com>
Subject: [PATCH V2 0/7] usb-misc: sisusbvga: cleanup and bug fix
Date: Fri, 15 Jan 2016 18:41:27 +0100	[thread overview]
Message-ID: <1452879694-21206-1-git-send-email-peter.senna@collabora.com> (raw)

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

             reply	other threads:[~2016-01-15 17:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-15 17:41 Peter Senna Tschudin [this message]
2016-01-15 17:41 ` [PATCH V2 1/7] usb-misc: sisusbvga: Fix coding style: horizontal whitespace changes Peter Senna Tschudin
2016-01-15 17:56   ` Joe Perches
2016-01-15 18:09     ` Peter Senna
2016-01-15 18:21       ` Alan Stern
2016-01-15 17:41 ` [PATCH V2 2/7] usb-misc: sisusbvga: Fix coding style: vertical " Peter Senna Tschudin
2016-01-15 17:41 ` [PATCH V2 3/7] usb-misc: sisusbvga: Fix coding style: braces, parenthesis, comment Peter Senna Tschudin
2016-01-15 17:41 ` [PATCH V2 4/7] usb-misc: sisusbvga: Fix coding style: remove assignment from if tests Peter Senna Tschudin
2016-01-15 17:41 ` [PATCH V2 5/7] usb-misc: sisusbvga: Remove null test before calls to kfree() Peter Senna Tschudin
2016-01-15 17:41 ` [PATCH V2 6/7] usb-misc: sisusbvga: Remove memory allocation logs Peter Senna Tschudin
2016-01-15 17:41 ` [PATCH V2 7/7] usb-misc: sisusbvga: fix error path Peter Senna Tschudin
2016-01-15 17:51 ` [PATCH V2 0/7] usb-misc: sisusbvga: cleanup and bug fix Joe Perches
2016-01-15 18:07   ` Jason Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1452879694-21206-1-git-send-email-peter.senna@collabora.com \
    --to=peter.senna@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=thomas@winischhofer.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox