Maybe an assert is really more appropriate, but technically doing so on
actual hardware should run flawlessly so I think the emu should work too...
Maybe I'm wrong though
On Tue, May 6, 2025, 19:48 Daniel P. Berrangé <berra...@redhat.com> wrote:
> On Tue, May 06, 2025 at 07:41:32PM +0300, Elisha Hollander wrote:
> > Gave an example for a case where QEMU would try to allocate 0 bytes thus
> > fail here in the original version of the patch.
> >
> > > As I mentioned earlier, let's say you don't initialize the vertical
> > display end registers, and set the minimum scanline register, the
> emulation
> > will then have to allocate some display buffer, but because the vertical
> > display end is initilized as 0 the buffer will be empty and the program
> > break.
>
> Isn't this an invalid hardware configuration that should be detected
> in the emulation code, and either force the display end to a minimum
> value, or trigger an assert ?
>
> Patching a bug in a specific HW impl, by changing the qemu_memfd_alloc
> code feels like it is probably the wrong place to address this.
>
> >
> > I have no idea as for why my emails are getting messed up... :/
> >
> > Have to go now, will try and send it again tomorrow probably...
> >
> > On Tue, May 6, 2025, 19:37 Daniel P. Berrangé <berra...@redhat.com>
> wrote:
> >
> > > On Tue, May 06, 2025 at 07:17:25PM +0300, Elisha Hollander wrote:
> > > > Sorry for former patch something is messed up with my email.
> > >
> > > The commit message needs to explain what problem is being solved by
> > > making this change as allowing 0 bytes looks dubious on the surface.
> > >
> > > >
> > > > Signed-off-by: donno2048 <just4now666...@gmail.com>
> > > > ---
> > > > util/memfd.c | 10 ++++++----
> > > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/util/memfd.c b/util/memfd.c
> > > > index 8a2e906..e96e5af 100644
> > > > --- a/util/memfd.c
> > > > +++ b/util/memfd.c
> > > > @@ -108,7 +108,7 @@ err:
> > > > void *qemu_memfd_alloc(const char *name, size_t size, unsigned int
> > > seals,
> > > > int *fd, Error **errp)
> > > > {
> > > > - void *ptr;
> > > > + void *ptr = NULL;
> > > > int mfd = qemu_memfd_create(name, size, false, 0, seals, NULL);
> > > >
> > > > /* some systems have memfd without sealing */
> > > > @@ -131,9 +131,11 @@ void *qemu_memfd_alloc(const char *name, size_t
> > > size,
> > > > unsigned int seals,
> > > > }
> > > > }
> > > >
> > > > - ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, mfd, 0);
> > > > - if (ptr == MAP_FAILED) {
> > > > - goto err;
> > > > + if (size != 0) {
> > > > + ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, mfd, 0);
> > > > + if (ptr == MAP_FAILED) {
> > > > + goto err;
> > > > + }
> > > > }
> > >
> > > This patch is mangled.
> > >
> > >
> > > With regards,
> > > Daniel
> > > --
> > > |: https://berrange.com -o-
> > > https://www.flickr.com/photos/dberrange :|
> > > |: https://libvirt.org -o-
> > > https://fstop138.berrange.com :|
> > > |: https://entangle-photo.org -o-
> > > https://www.instagram.com/dberrange :|
> > >
> > >
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o-
> https://www.instagram.com/dberrange :|
>
>
