Re: [PATCH kvm-unit-tests 06/17] pci: introduce struct pci_dev

From: Peter Xu <peterx@redhat.com>
To: Andrew Jones <drjones@redhat.com>
Cc: kvm@vger.kernel.org, rkrcmar@redhat.com, agordeev@redhat.com,
	jan.kiszka@web.de, pbonzini@redhat.com
Subject: Re: [PATCH kvm-unit-tests 06/17] pci: introduce struct pci_dev
Date: Mon, 7 Nov 2016 14:42:29 -0500	[thread overview]
Message-ID: <20161107194229.GN3719@pxdev.xzpeter.org> (raw)
In-Reply-To: <20161107180212.pcidrbzzvqwfdda6@kamzik.brq.redhat.com>

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?

[...]

> > 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,

-- peterx