On Mon, 1 Aug 2011, Luis R. Rodriguez wrote:
> On Mon, Aug 1, 2011 at 1:38 PM, Pavel Roskin <proski@xxxxxxx> wrote:
> > On 08/01/2011 03:04 PM, Compat-wireless cronjob account wrote:
> >>
> >> From git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next
> >> + d7c6e83...c39a889 akpm-end -> origin/akpm-end (forced update)
> >> 55f9c40..3da3f87 akpm-start -> origin/akpm-start
> >> + cf684c9...346346f master -> origin/master (forced update)
> >> 55f9c40..3da3f87 stable -> origin/stable
> >> * [new tag] next-20110801 -> next-20110801
> >> /usr/bin/sha1sum: *.tar.bz2: No such file or directory
> >
>
> Right, this failed to build the compat-wireless daily tarball based on
> linux-next.git due to a failed hunk:
>
> Applying backport patch: patches/01-netdev.patch
> patching file drivers/net/usb/rndis_host.c
> patching file drivers/net/usb/usbnet.c
> patching file drivers/net/wireless/rndis_wlan.c
> patching file net/mac80211/iface.c
> Hunk #1 FAILED at 698.
> Hunk #2 succeeded at 846 (offset 1 line).
> Hunk #3 succeeded at 885 (offset 1 line).
> Hunk #4 succeeded at 1136 (offset 1 line).
> Hunk #5 succeeded at 1146 (offset 1 line).
> 1 out of 5 hunks FAILED -- saving rejects to file net/mac80211/iface.c.rej
> patching file drivers/net/b44.c
> patching file net/wireless/wext-core.c
> patching file drivers/net/wireless/ipw2x00/ipw2100.c
> patching file drivers/net/wireless/ipw2x00/ipw2200.c
> patching file drivers/net/wireless/iwmc3200wifi/netdev.c
> patching file drivers/net/wireless/libertas/main.c
> patching file drivers/net/wireless/libertas/mesh.c
> patching file drivers/net/wireless/libertas/defs.h
> patching file drivers/net/wireless/mac80211_hwsim.c
> patching file drivers/net/wireless/mwifiex/main.c
> patching file drivers/net/wireless/orinoco/main.c
> patching file net/bluetooth/bnep/netdev.c
> Hunk #2 FAILED at 235.
> 1 out of 2 hunks FAILED -- saving rejects to file
> net/bluetooth/bnep/netdev.c.rej
> patching file drivers/net/atl1e/atl1e_main.c
> patching file drivers/net/atl1c/atl1c_main.c
> patching file drivers/net/atlx/atl1.c
> patching file drivers/net/atlx/atl2.c
> Patching patches/01-netdev.patch failed, update it
>
> Whenever we have a failed hunk we run into this. I think we can do
> better, we could break down 01-netdev.patch into one a coccinelle
> spatch for example and then the rest to be handled separately, this
> may help speed with making us lazier and making the backup even more
> automatic.
>
> For example:
>
> mcgrof@tux ~/linux-next (git::master)$ cat netdev-attach.cocci
> @@
> expression ptr, ops;
> @@
> ...
> -ptr->netdev_ops = ops;
> +netdev_attach_ops(ptr, ops);
> ...
The outer dots are not needed. Even incorrect, because they imply that
there is only one occurrence of this code in the current function.
> mcgrof@tux ~/linux-next (git::master)$ spatch -sp_file
> netdev-attach.cocci -in_place drivers/net/usb/rndis_host.c
> init_defs_builtins: /usr/share/coccinelle/standard.h
> HANDLING: drivers/net/usb/rndis_host.c
> diff =
> --- drivers/net/usb/rndis_host.c 2011-04-25 11:44:25.713248001 -0700
> +++ /tmp/cocci-output-14779-d7a728-rndis_host.c 2011-08-01
> 16:30:48.342283464 -0700
> @@ -355,7 +355,7 @@ generic_rndis_bind(struct usbnet *dev, s
> dev->rx_urb_size &= ~(dev->maxpacket - 1);
> u.init->max_transfer_size = cpu_to_le32(dev->rx_urb_size);
>
> - net->netdev_ops = &rndis_netdev_ops;
> + netdev_attach_ops(net, &rndis_netdev_ops);
>
> retval = rndis_command(dev, u.header, CONTROL_BUFFER_SIZE);
> if (unlikely(retval < 0)) {
>
> Question then is -- are we willing to require spatch for building the
> tarballs? I'm OK with this, anyone else?
The problem with Coccinelle is that there can be false positives and false
negatives, ie th rule applies where it should not and doesn't apply where
it should. But there are probably cases like this one where one knows
that it should always be done this way. It would be nice, though if there
could be some kind of guarantee that the right thing has happened.
Perhaps there could be a way to generate from a semantic patch some checks
that it has applied everywhere possible. Or perhaps the semantic patch
could be only used as a last resort, if the normal application fails. In
that way there would be less to check by hand.
julia
