[PATCH:libpciaccess] Solaris support for multiple PCI segments (domains)

[Adam Jackson]

On Tue, 2011-03-29 at 15:07 -0400, John Martin wrote:
> On 03/29/11 02:51 PM, Adam Jackson wrote:
> > On Tue, 2011-03-29 at 07:51 -0700, Alan Coopersmith wrote:
> >
> >> +typedef struct probe_info {
> >> +    volatile size_t num_allocated_elems;
> >> +    volatile size_t num_devices;
> >> +    struct pci_device_private * volatile devices;
> >> +} probe_info_t;
> >
> > I'm virtually certain that 'volatile' is not useful here.
> 
> In pci_system_solx_devfs_create() the probe_info structure
> is passed to di_walk_minor() and each element can returned
> changed by the dynamic resizing code in probe_dev().

int di_walk_minor(di_node_t root, const char *minor_nodetype,
    uint_t flag, void *arg, int (*minor_callback)(di_node_t node,
    di_minor_t minor, void *arg));

The probe_info_t is the 'void *arg' there.  Since it is not marked const
in any way - either in the prototype of the callee, or in the variable
decl itself - the compiler must assume di_walk_minor may modify the data
it points to.  Adding 'volatile' is not necessary. [1]

Consider, as a counterexample, that there exist only two variables in
the core X server that are volatile, and yet we do a very similar
resize-the-array move all the time.  Is this because X is broken, or is
this because your understanding of volatile is faulty?

[1] - Unless, of course, di_walk_minor is handing that memory to
something that runs concurrently with the pciaccess consumer, and may
modify it asynchronously.  If that's true, though, that's one hell of a
broken API.

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110329/824f4613/attachment.pgp>