On Tue, 28 Jun 2022 09:22:13 -0700
Tyler Retzlaff <roret...@linux.microsoft.com> wrote:
> On Tue, Jun 28, 2022 at 04:46:43PM +0200, David Marchand wrote:
> > Make rte_bus opaque for non internal users.
> > This will make extending this object possible without breaking the ABI.
> >
> > Introduce a new driver header and move rte_bus definition and helpers.
> >
> > Signed-off-by: David Marchand <david.march...@redhat.com>
> > ---
>
> ... snip ...
>
> > -struct rte_bus {
> > - RTE_TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
> > - const char *name; /**< Name of the bus */
> > - rte_bus_scan_t scan; /**< Scan for devices attached to bus */
> > - rte_bus_probe_t probe; /**< Probe devices on bus */
> > - rte_bus_find_device_t find_device; /**< Find a device on the bus */
> > - rte_bus_plug_t plug; /**< Probe single device for drivers */
> > - rte_bus_unplug_t unplug; /**< Remove single device from driver */
> > - rte_bus_parse_t parse; /**< Parse a device name */
> > - rte_bus_devargs_parse_t devargs_parse; /**< Parse bus devargs */
> > - rte_dev_dma_map_t dma_map; /**< DMA map for device in the bus */
> > - rte_dev_dma_unmap_t dma_unmap; /**< DMA unmap for device in the bus */
> > - struct rte_bus_conf conf; /**< Bus configuration */
> > - rte_bus_get_iommu_class_t get_iommu_class; /**< Get iommu class */
> > - rte_dev_iterate_t dev_iterate; /**< Device iterator. */
> > - rte_bus_hot_unplug_handler_t hot_unplug_handler;
> > - /**< handle hot-unplug failure on the bus */
> > - rte_bus_sigbus_handler_t sigbus_handler;
> > - /**< handle sigbus error on the bus */
> > -
> > -};
>
> since we're overhauling anyway we could follow suit with a number of the
> lessons from posix apis e.g. pthread and eliminate typed pointers for a
> little extra value.
>
> > +struct rte_bus;
> > +struct rte_device;
>
> to avoid people tripping over mishandling pointers in/out of the api
> surface taking the opaque object you could declare opaque handle for the
> api to operate on instead. it would force the use of a cast in the
> implementation but would avoid accidental void * of the wrong thing that
> got cached being passed in. if the cast was really undesirable just
> whack it under an inline / internal function.
I don't like that because it least to dangerous casts in the internal code.
Better to keep the the type of the object. As long as the API only passes
around an pointer to a struct, without exposing the contents of the struct;
it is safer and easier to debug.
