linux-kernel - Re: [GIT PATCH] USB patches for 2.6.33-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 23 Dec 2009 15:38:27 -0800
From:	Greg KH <gregkh@...e.de>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [GIT PATCH] USB patches for 2.6.33-rc1

On Wed, Dec 23, 2009 at 01:48:09PM -0800, Linus Torvalds wrote:
> 
> 
> On Wed, 23 Dec 2009, Greg KH wrote:
> > 
> > They do:
> > 	- reorg the device ids in one usb-serial driver.  I took this
> > 	  now as it's a merge pressure point, and this is just a code
> > 	  move now, no functionality changed at all.  It's odd that git
> > 	  didn't show this as a code move in the diffstat, I used:
> > 	  	git diff -m --stat --summary
> > 	  to generate the diffstat.  Should I be doing something else?
> > 	  Or is it because the code is being removed from one file and
> > 	  placed in another one?
> 
> Since the old file stays around, git doesn't consider it a rename.
> 
> Now, you can actually change that a bit. You can use -C instead of -M, 
> which enables copy-detection rather than just renames, and then it changes 
> from
> 
>  drivers/usb/serial/ftdi_sio.c           |    1 +
>  drivers/usb/serial/ftdi_sio.h           |  959 +------------------------------
>  drivers/usb/serial/ftdi_sio_ids.h       |  986 +++++++++++++++++++++++++++++++
> 
> to
> 
>  drivers/usb/serial/ftdi_sio.c                     |    1 +
>  drivers/usb/serial/ftdi_sio.h                     |  959 +-----------
>  drivers/usb/serial/{ftdi_sio.h => ftdi_sio_ids.h} | 1785 ++++++++-------------
>  copy drivers/usb/serial/{ftdi_sio.h => ftdi_sio_ids.h} (65%)
> 
> which isn't actually any better (since while it notices the copy of the 
> data, it _removes_ all the code whily copying, so the total number of 
> lines actually goes up!).
> 
> So -M doesn't really "help" in that the diff itself does end up larger, 
> but at the same time it is somewhat informative in the sense that you do 
> see where the data comes from.
> 
> To see another facet of it all, you could also use "-B" to say that you 
> want to Break all old name associations if a file has changed a lot, and 
> then enable rename (or copy) detection, and get
> 
>  drivers/usb/serial/ftdi_sio.c                     |    1 +
>  drivers/usb/serial/ftdi_sio.h                     | 2063 ++++++---------------
>  drivers/usb/serial/{ftdi_sio.h => ftdi_sio_ids.h} | 1785 +++++++------------
>  rewrite drivers/usb/serial/ftdi_sio.h (68%)
>  rename drivers/usb/serial/{ftdi_sio.h => ftdi_sio_ids.h} (65%)
> 
> which is the largest of all diffs but it shows another side of the 
> situation: that sio.h file has is now a "rewrite", and the end result is 
> substantially different from the original. So git shows it as a "rename in 
> place" where the diff shows up as a total delete followed by a total 
> addition.
> 
> As you can see, git does see that you moved data around, but since a lot 
> of the data also _stayed_ in the original file, the copy/break operations 
> really don't help. You cannot show "copy this part of this file into that 
> other file" as a diff.
> 
> Btw, we _do_ do the "copy this part of this file into that other file" 
> when you do "git blame -C", so you can now do
> 
> 	git blame -C drivers/usb/serial/ftdi_sio_ids.h
> 
> and it will follow the history of the lines in that file back to the 
> ftdi_sio.h file. So the fact that git doesn't show it in the diffs is 
> really because diffs always work on a whole-file basis (ie you can show a 
> "rename file" diff, but you can't show a "move one function or a set of 
> ID's from one file to another" diff.

Thanks for the through explaination.  I guess I'll just stick with '-M'
on the diffstat summaries for now, but it is good to know that git can
track this type of change through the changeset, which is the most
important thing.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/