On Mon, Nov 07, 2016 at 02:42:29PM -0500, Peter Xu wrote: > On Mon, Nov 07, 2016 at 07:02:12PM +0100, Andrew Jones wrote: > > [...] > > > > I thought pcidevaddr_t means bdf? That's a wild guess of me from its > > > size, as it's defined as: > > > > > > typedef uint16_t pcidevaddr_t; > > > > > > uint16_t makes it looks more like bdf rather than devfn. For devfn, > > > uint8_t suites better. > > > > > > And what I mean here is to pass in the bdf of the device, not devfn > > > only (in our case, devfn is always bdf though, since we are only > > > supporting bus number zero). > > > > see lib/x86/asm/pci.h:pci_config_read how 'dev' gets used, it's only dev. > > But shouldn't "dev" the same as "bdf" here? Hmm, yes. We're only shifting it by 8, not 11, so you're right. The word 'dev' in pcidevaddr_t threw me off and I didn't bother looking up CF8's bits until just now. OK, let's try to get consistent naming going on, and we need better comments (comments at all). I'd like to see one briefly describing 'bdf' and then use that for the primary name, i.e. rename pcidevaddr_t to something like 'pcibdfaddr_t' or whatever. > > [...] > > > > Yes, actually this is something of my own "bad" habit. I used to add > > > xxx_ prefix for better tag generations. When I read some big C repos > > > like Linux, I see lots of duplicated field names for different > > > structs/unions. For example, when I want to see codes related to > > > StructA.FieldX, cscope will always tell me something more than this > > > (rather than giving me StructA.FieldX, it gives me > > > Struct[ABCDE..].FieldX). Then I need to pick out where StructA is. > > > That's sometimes annoying, which depends on the length of the tag > > > list. So I prefer to add this kind of prefix in the codes. > > > > > > Actually when I read some other codes (IIUC FreeBSD kernel), I see > > > that this "bad" technique is used as well with very short prefix, > > > e.g., for a struct called "net_filter" and field called "users", it > > > may be defined as: > > > > > > net_filter.nf_users > > > > > > So the "nf_" prefix is the short form of "net_filter", and it can be > > > used to distinguish the net_filter "users" fields with other "users" > > > ones. > > > > > > Here "pci" is short enough IMHO, so I prefixed it. > > > > > > Not sure whether it's a good idea though. > > > > Seems like overkill for kvm-unit-tests. > > Ok. Let me remove all pci_ prefix for pci_dev struct fields. > Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
