On Fri, 30 Nov 2018 at 20:47, Corey Minyard <miny...@acm.org> wrote: > > On 11/30/18 11:39 AM, Peter Maydell wrote: > > On Mon, 26 Nov 2018 at 20:04, <miny...@acm.org> wrote: > >> From: Philippe Mathieu-Daudé <phi...@redhat.com> > >> /* XXX: make this persistent */ > >> - uint8_t *eeprom_buf = g_malloc0(8 * SMBUS_EEPROM_SIZE); > >> + eeprom_buf = g_malloc0(nb_eeprom * SMBUS_EEPROM_SIZE); > > So if we allocate N buffers as the caller requests, what > > is the thing that means that more than 8 won't work ? > > > > We've now changed from allocating always 8 lots of > > the EEPROM size to possibly allocating fewer than that. > > How does the code in the device know what the size > > of the buffer we're passing as the "data" property is > > now? We don't pass it the number of EEPROMs as a property. > > It doesn't have to. Each EEPROM is 256 bytes and that's all the data > it has. > > But this whole thing is confusing, I agree. The more I look at this > particular file, the less I like it. But the caller is basically saying: > "I need N EEPROMs, here's the initialization data". That's not > really very flexible, IMHO the EEPROM devices should be created > individually using standard qemu methods.
Oh, yes, I see now. We pass in a block of N * 256 bytes, and the function then hands a pointer to offsets 0, 256, ... to each device it creates. I definitely don't see why we need to say "only a maximum of 8" now, though -- where does that limit come from? If we get passed in an arbitrary number of EEPROMs X, and we allocate 256 * X bytes, and create X devices which each get one slice of the buffer, what goes wrong when X > 8 ? > I'm tempted to rewrite this whole thing to make it cleaner. It's certainly pretty awkward code. I think personally given this patchset is already pretty big I'd go for getting it into master first and then doing a followup with further cleanup. thanks -- PMM
