[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devi
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host |
Date: |
Tue, 2 Feb 2016 17:31:11 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Tue, Feb 02, 2016 at 12:28:24PM -0500, Programmingkid wrote:
>
> On Feb 2, 2016, at 12:16 PM, Daniel P. Berrange wrote:
>
> > On Tue, Feb 02, 2016 at 12:08:31PM -0500, Programmingkid wrote:
> >> https://patchwork.ozlabs.org/patch/570128/
> >>
> >> Mac OS X can be picky when it comes to allowing the user
> >> to use physical devices in QEMU. Most mounted volumes
> >> appear to be off limits to QEMU. If an issue is detected,
> >> a message is displayed showing the user how to unmount a
> >> volume. Now QEMU uses both CD and DVD media.
> >>
> >> Signed-off-by: John Arbuckle <address@hidden>
> >>
> >> ---
> >> Changed filename variable to a character array.
> >> Changed how filename was set to bsd_path's value by using snprintf().
> >
> > Whats the rationale here ? Using pre-allocated fixed
> > length arrays is pretty bad practice in general, but
> > especially so for filenames
>
> With an automatic variable there is no worry about when to release it.
>
> >
> >
> >> @@ -2119,34 +2173,59 @@ static int hdev_open(BlockDriverState *bs, QDict
> >> *options, int flags,
> >> int ret;
> >>
> >> #if defined(__APPLE__) && defined(__MACH__)
> >> - const char *filename = qdict_get_str(options, "filename");
> >
> > [snip]
> >
> >> + char filename[MAXPATHLEN];
> >> + bool error_occurred = false;
> >> + snprintf(filename, MAXPATHLEN, "%s", qdict_get_str(options,
> >> "filename"));
> >
> > This is a step backwards compared to the original code
> > IMHO. There's no good reason to use a stack allocated
> > fixed array here - the code that follows would be
> > quite happy with the original 'const char *'.
>
> Having to decide when and where to free memory is eliminated with
> automatic variables. The code looks cleaner without all the g_free()'s.
> It also might eliminate possible memory leaks.
There was/is no leak because it qdict_get_str() returns 'const char *' and
so nothing needs freeing. So your change is still a backwards steps IMHO.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
- [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Programmingkid, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Daniel P. Berrange, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Programmingkid, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host,
Daniel P. Berrange <=
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Programmingkid, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Eric Blake, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Programmingkid, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Programmingkid, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Eric Blake, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Programmingkid, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Eric Blake, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Programmingkid, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Eric Blake, 2016/02/02
- Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host, Programmingkid, 2016/02/03