[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 1/3] monitor/hmp: add support for flag argument with value
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH v8 1/3] monitor/hmp: add support for flag argument with value |
Date: |
Thu, 24 Feb 2022 11:30:17 +0000 |
User-agent: |
Mutt/2.1.5 (2021-12-30) |
* Markus Armbruster (armbru@redhat.com) wrote:
> Fabian Ebner <f.ebner@proxmox.com> writes:
>
> > Am 09.02.22 um 15:12 schrieb Markus Armbruster:
> >> Fabian Ebner <f.ebner@proxmox.com> writes:
> >
> > ----8<----
> >
> >>> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> >>> index 3da3f86c6a..a4cb307c8a 100644
> >>> --- a/monitor/monitor-internal.h
> >>> +++ b/monitor/monitor-internal.h
> >>> @@ -63,7 +63,8 @@
> >>> * '.' other form of optional type (for 'i' and 'l')
> >>> * 'b' boolean
> >>> * user mode accepts "on" or "off"
> >>> - * '-' optional parameter (eg. '-f')
> >>> + * '-' optional parameter (eg. '-f'); if followed by a 'V', it
> >>> + * specifies an optional string param (e.g. '-fV' allows
> >>> '-f foo')
> >>> *
> >>> */
> >>
> >> For what it's worth, getopt() uses ':' after the option character for
> >> "takes an argument".
> >>
> >
> > Doing that leads to e.g.
> > .args_type = "protocol:s,password:s,display:-d:,connected:s?",
> > so there's two different kinds of colons now.
>
> Point.
Yeh, : is probably a bad idea.
> > It's not a problem
> > functionality-wise AFAICT, but it might not be ideal. Should I still go
> > for it?
> >
> > Also, wouldn't future non-string flag parameters need their own letter
> > too? What about re-using 's' here instead?
>
> Another good point.
>
> Dave, what do you think?
Yeh, I'd be happy reusing 's' here.
Dave
> >> Happy to make that tweak in my tree. But see my review of PATCH 3
> >> first.
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
[PATCH v8 3/3] qapi/monitor: allow VNC display id in set/expire_password, Fabian Ebner, 2022/02/04