On Tue, 18 Jul 2023 14:10:24 +0200 Philippe Mathieu-Daudé <[email protected]> wrote:
> On 18/7/23 12:13, Peter Maydell wrote: > > In build_cdat_table() we do: > > *cdat_table = g_malloc0(sizeof(*cdat_table) * CXL_USP_CDAT_NUM_ENTRIES); > > This is wrong because: > > - cdat_table has type CDATSubHeader *** > > Yes > > > - so *cdat_table has type CDATSubHeader ** > > Yes > > > - so the array we're allocating here should be items of type > > CDATSubHeader * > > Yes > > > - but we pass sizeof(*cdat_table), which is sizeof(CDATSubHeader **), > > Indeed > > > implying that we're allocating an array of CDATSubHeader ** > > Ouch > > > It happens that sizeof(CDATSubHeader **) == sizeof(CDATSubHeader *) > > Ah! > > > so nothing blows up, but this should be sizeof(**cdat_table). > > Still, what a mess :) > > > Avoid this excessively hard-to-understand code by using > > g_new0() instead, which will do the type checking for us. > > While we're here, we can drop the useless check against failure, > > as g_malloc0() and g_new0() never fail. > > Reviewed-by: Philippe Mathieu-Daudé <[email protected]> > > > This fixes Coverity issue CID 1508120. > > > > Signed-off-by: Peter Maydell <[email protected]> > > --- > > Disclaimer: I have not tested this beyond any testing you > > get from 'make check' and 'make check-avocado'. Ah. Had this on my todo list form your report but got distracted. Anyhow, thanks for fixing it up. Tested somewhat. Actually poking this particular DOE instance to read the CDAT requires a bunch of kernel hacks I haven't forward ported recently and I don't want to delay this until I get back to them - depends on some core PCI subsystem work that is ongoing. Code isn't dependent on reading though so I can't see how this would break anything. Reviewed-by: Jonathan Cameron <[email protected]> > > --- > > hw/pci-bridge/cxl_upstream.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) >
