Re: [Qemu-devel] [PATCH] qemu-io: initialize progname with error_set_progname()

[Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>]

At Mon, 11 Aug 2014 13:20:38 +0200,
Markus Armbruster wrote:
> 
> Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> writes:
> 
> > 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.
> >
> > Cc: Kevin Wolf <kwolf@redhat.com>
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
> > ---
> >  qemu-io.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/qemu-io.c b/qemu-io.c
> > index b55a550..6f08a91 100644
> > --- a/qemu-io.c
> > +++ b/qemu-io.c
> > @@ -395,6 +395,7 @@ int main(int argc, char **argv)
> >  #endif
> >  
> >      progname = basename(argv[0]);
> > +    error_set_progname(progname);
> >      qemu_init_exec_dir(argv[0]);
> >  
> >      while ((c = getopt_long(argc, argv, sopt, lopt, &opt_index)) != -1) {
> 
> Global variable progname could be replaced by error_get_progname().

Hi Markus, thanks for your comments.

Yes, replacing is possible but I think we don't have to do it. The
main purpose of error_set_progname() is improving output of
error_report(). If program uses its binary name frequently, caching it
in other variable is suitable.

> 
> Can pass argv[0] directly, without basename().

Yes, as Stefan pointed, I'll do so in v2.

> 
> Do any other programs besides qemu-io and qemu proper need to call
> error_set_progname()?  Quick grep for main() finds
> 
>     qemu-bridge-helper.c
>     qemu-io.c
>     qemu-nbd.c
>     bsd-user/main.c
>     disas/m68k.c
>     fsdev/virtfs-proxy-helper.c
>     libcacard/link_test.c
>     libcacard/vscclient.c
>     linux-user/main.c
>     qga/main.c
>     target-ppc/mfrom_table_gen.c
>     pc-bios/s390-ccw/main.c
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

I think
    qemu-io.c
    qemu-nbd.c
    bsd-user/main.c
    linux-user/main.c
    qga/main.c
should call error_get_progname() because they seem to call
error_get_progname() potentially. But others seem to be simple
utilities so calling the func might be needless.

I'll update the above utilities in v2 (except bsd-user/main.c, because
I don't have BSD environment and cannot check the change:( ).

BTW, should I split patches for each program? They seem to be
maintained by different maintainers. But the change is really trivial
so packing them in one patch is also reasonable. I'd like to hear your
opinion.

Thanks,
Hitoshi