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] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 16 Oct 2007 19:21:53 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Al Viro <viro@....linux.org.uk>
cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fix adbhid mismerge



On Tue, 16 Oct 2007, Linus Torvalds wrote:
> 
> I don't think you did anything wrong. You used both --full-history 
> (implicitly: git-whatchanged) and you made sure to see the diffs for both 
> sides of any merge (-m), and that means that you should see every single 
> diff involved.

Btw, if anybody can come up with a better way to find these kinds of 
mis-merges, I'd love to hear about it.

In *this* particular case, the -c flag ("combined" merge diff) probably 
comes closest, and is certainly a lot better than passing in -m (which 
shows each merge against both parents separately), and in fact, I think 
you would have found the mis-merge immediately if you had used

	git whatchanged -p -c v2.6.23.. drivers/macintosh/adbhid.c

but I'm not going to guarantee that -c always gives you what you want.

In general, the rules are:

 - the default for merge diffs is to show "condensed combined" merge, ie 
   the diff of only those parts where the result actively differs from 
   *both* parents.

   This is very terse, and it has the wonderful property of showing merges 
   where you actually ended up doing "real work" and not just picking one 
   side or the other, but in this case the very fact that the mis-merge 
   had picked one side (and it really would have _needed_ a correct manual 
   merge) also meant that the default "--cc" format didn't show anything 
   at all.

 - "-c" is for regular combined merges: any file that was modified in both 
   parents will show up as a combination of the diffs of both sides, while 
   a file that was taken in its *entirety* is ignored.

   In this case that's exactly what you wanted. It's just too noisy to 
   necessarily be the default, and you can still have a silent mis-merge 
   if the merger picked *only* one side.

   But in general, I suspect that "-c" is often a good thing to try if you 
   cannot find the cause of some change in a regular commit, and suspect a 
   merge error.

 - "-m" shows each side totally independently. Quite frankly, I've never 
   found it useful. It is essentially guaranteed to show all changes, 
   since it shows the patches against all parents individually, so even if 
   we took only one side, we'll still show the patch against the *other* 
   side, but quite frankly, while it's thus useful in theory, in practice 
   the end result is just too noisy to likely ever really be useful as 
   anything but a "yes, the information is there (..but it's practically 
   impossible to find for all the other noise that is also there)"

The main reason "-m" exists is historical: before Junio implemented the 
combined formats, -m was the easy way to show *any* information. I bet -m 
can be useful in some case where you have some pattern you can search for 
(ie I used -m in this case to find the mis-merge, but realized only later 
that I would have been better off using -c), but it's not something I'd 
recommend unless you were really desperate.

What I'd actually really like would be something that shows the original 
conflict, but that's really expensive to compute (it basically involves 
re-doing the merge from scratch - finding the proper base commit(s) etc). 
So we never did that.

		Linus
-
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ