linux-kernel - Re: [PATCH 0/5 v2] PCI: fix cardbus and sriov regressions

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:	Sun, 3 Jul 2011 14:30:00 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Ram Pai <linuxram@...ibm.com>
Cc:	jbarnes@...tuousgeek.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, yinghai@...nel.org,
	bhutchings@...arflare.com, socketcan@...tkopp.net,
	bhelgaas@...gle.com, linux@...inikbrodowski.net
Subject: Re: [PATCH 0/5 v2] PCI: fix cardbus and sriov regressions

Gaah. I'm still rather uncomfortable about this, and I wonder about
patch 2 in particular. It seems that that patch could/should be split
up: the whole change to "find_resource()" etc looks like prime
material for a separate patch that splits up that function and
explains why that change is done.

Also, quite frankly, by the time you pass in eight different arguments
(and pretty complex ones at that, with one being the alignment
function pointer), I start thinking that you should have passed in a
pointer to a descriptor structure instead. I get the feeling that the
"resource requirements" really should be a structure instead of lots
of individual arguments:

IOW, this part:

+                     resource_size_t newsize, resource_size_t min,
+                     resource_size_t max, resource_size_t align,
+                     resource_size_t (*alignf)(void *,
+                                               const struct resource *,
+                                               resource_size_t,
+                                               resource_size_t),
+                     void *alignf_data)

really makes me go

   struct resource_requirement {
      resource_size_t min, max, align;
      resource_size_t (*alignf)(const struct resource *, struct
resource_requirement *);
      void *alignf_data);
   };

and I'd really change the function argument to take that kind of
simplified thing instead.

And that cleanup/re-organization would be prime material for a totally
independent patch that changes no semantics at all, just prepares for
the other changes.

That way the final "patch 2" would be smaller and do the semantic
changes, instead of being a mix of semantic changes and infrastructure
changes.

And some of the cleanup stuff I could merge for 3.0 just to make things easier.

Hmm?

                 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/