diff mbox

[v2,1/4] qemu-io: initialize progname with error_set_progname()

Message ID 1421917694-6824-2-git-send-email-mitake.hitoshi@lab.ntt.co.jp
State New
Headers show

Commit Message

Hitoshi Mitake Jan. 22, 2015, 9:08 a.m. UTC
Calling error_get_progname() in the context of qemu-io can cause
segmentation fault because qemu-io doesn't initialize its progname
with error_set_progname(). This patch adds the initialization.

Currently, the missing call of error_set_progname() doesn't cause any
problems because qemu-io doesn't use error_get_progname(). This patch
is a proactive action.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
---
 qemu-io.c | 1 +
 1 file changed, 1 insertion(+)

v2:
 - pass argv[0] directly to error_set_progname() without calling basename

Comments

Hitoshi Mitake Feb. 2, 2015, 9:51 a.m. UTC | #1
At Thu, 22 Jan 2015 18:08:11 +0900,
Hitoshi Mitake wrote:
> 
> Calling error_get_progname() in the context of qemu-io can cause
> segmentation fault because qemu-io doesn't initialize its progname
> with error_set_progname(). This patch adds the initialization.
> 
> Currently, the missing call of error_set_progname() doesn't cause any
> problems because qemu-io doesn't use error_get_progname(). This patch
> is a proactive action.
> 
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
> ---
>  qemu-io.c | 1 +
>  1 file changed, 1 insertion(+)

Hi Kevin, Stefan, could you pick this patch? sheepdog driver has a
pending patch [1] which depends on it. I believe at least qemu-io
should call error_set_progname() because block drivers can use the
qemu-error infrastructure.

[1] https://github.com/sheepdog/qemu/commit/a95c35e606a2a189e7dbaf645277c5c306b01e4b

Thanks,
Hitoshi

> 
> v2:
>  - pass argv[0] directly to error_set_progname() without calling basename
> 
> diff --git a/qemu-io.c b/qemu-io.c
> index 91a445a..0e7d7d4 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -396,6 +396,7 @@ int main(int argc, char **argv)
>  
>      progname = basename(argv[0]);
>      qemu_init_exec_dir(argv[0]);
> +    error_set_progname(argv[0]);
>  
>      bdrv_init();
>  
> -- 
> 1.9.1
>
Kevin Wolf Feb. 2, 2015, 11:36 a.m. UTC | #2
Am 02.02.2015 um 10:51 hat Hitoshi Mitake geschrieben:
> At Thu, 22 Jan 2015 18:08:11 +0900,
> Hitoshi Mitake wrote:
> > 
> > Calling error_get_progname() in the context of qemu-io can cause
> > segmentation fault because qemu-io doesn't initialize its progname
> > with error_set_progname(). This patch adds the initialization.
> > 
> > Currently, the missing call of error_set_progname() doesn't cause any
> > problems because qemu-io doesn't use error_get_progname(). This patch
> > is a proactive action.
> > 
> > Cc: Kevin Wolf <kwolf@redhat.com>
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
> > ---
> >  qemu-io.c | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Hi Kevin, Stefan, could you pick this patch? sheepdog driver has a
> pending patch [1] which depends on it. I believe at least qemu-io
> should call error_set_progname() because block drivers can use the
> qemu-error infrastructure.

So are you planning to get the individual patches merged on their own
instead of the whole series as one?

> [1] https://github.com/sheepdog/qemu/commit/a95c35e606a2a189e7dbaf645277c5c306b01e4b

That patch looks wrong. Nobody guarantees that qemu-io is really called
qemu-io. The user can name their binaries as they want and create
symlinks with any name, and indeed names for qemu-img used by some
distributions include 'qemu-img-kvm' and 'kvm-img'. You need to find a
different way than checking binary file names.

Kevin
Hitoshi Mitake Feb. 2, 2015, 2:04 p.m. UTC | #3
At Mon, 2 Feb 2015 12:36:00 +0100,
Kevin Wolf wrote:
> 
> Am 02.02.2015 um 10:51 hat Hitoshi Mitake geschrieben:
> > At Thu, 22 Jan 2015 18:08:11 +0900,
> > Hitoshi Mitake wrote:
> > > 
> > > Calling error_get_progname() in the context of qemu-io can cause
> > > segmentation fault because qemu-io doesn't initialize its progname
> > > with error_set_progname(). This patch adds the initialization.
> > > 
> > > Currently, the missing call of error_set_progname() doesn't cause any
> > > problems because qemu-io doesn't use error_get_progname(). This patch
> > > is a proactive action.
> > > 
> > > Cc: Kevin Wolf <kwolf@redhat.com>
> > > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > > Cc: Markus Armbruster <armbru@redhat.com>
> > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
> > > ---
> > >  qemu-io.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > 
> > Hi Kevin, Stefan, could you pick this patch? sheepdog driver has a
> > pending patch [1] which depends on it. I believe at least qemu-io
> > should call error_set_progname() because block drivers can use the
> > qemu-error infrastructure.
> 
> So are you planning to get the individual patches merged on their own
> instead of the whole series as one?

I thought this patchset can be proactive fix. So I'm separating the
patch for sheepdog driver from this one.

> 
> > [1] https://github.com/sheepdog/qemu/commit/a95c35e606a2a189e7dbaf645277c5c306b01e4b
> 
> That patch looks wrong. Nobody guarantees that qemu-io is really called
> qemu-io. The user can name their binaries as they want and create
> symlinks with any name, and indeed names for qemu-img used by some
> distributions include 'qemu-img-kvm' and 'kvm-img'. You need to find a
> different way than checking binary file names.

I couldn't consider about the case, thanks for pointing! I'll seek
other method for distinguishing binary files.

Thanks,
Hitoshi

> 
> Kevin
diff mbox

Patch

diff --git a/qemu-io.c b/qemu-io.c
index 91a445a..0e7d7d4 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -396,6 +396,7 @@  int main(int argc, char **argv)
 
     progname = basename(argv[0]);
     qemu_init_exec_dir(argv[0]);
+    error_set_progname(argv[0]);
 
     bdrv_init();