Re: [libvirt] [PATCH v3 4/8] vsh: Extract vshCmddefCheckInternals from vshCmddefOptParse
On 16/09/16 13:58, Ján Tomko wrote: > On Fri, Sep 16, 2016 at 12:50:41PM +0200, Erik Skultety wrote: >> Originally introduced by commit 2432521e which correctly split >> vshCmddefOptParse into command's options validation and options parsing. >> However, command's 'internals' are not tied solely to .options, rather it >> should be about the overall structure, therefore the validation should be >> extracted from vshCmddefOptParse and performed before any attempt to >> parse the >> command's options. >> >> Signed-off-by: Erik Skultety>> --- >> tools/vsh.c | 18 +- >> 1 file changed, 13 insertions(+), 5 deletions(-) >> > > ACK > > While this change makes sense on its own, this function is only used for > a run-time check of our data. Maybe the only place to call it should be > the self test command? > > Jan I think we could do this, since not paying attention to the results of our test suite would be developer's ignorance, so I'll update the patch. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/8] vsh: Extract vshCmddefCheckInternals from vshCmddefOptParse
On Fri, Sep 16, 2016 at 12:50:41PM +0200, Erik Skultety wrote: Originally introduced by commit 2432521e which correctly split vshCmddefOptParse into command's options validation and options parsing. However, command's 'internals' are not tied solely to .options, rather it should be about the overall structure, therefore the validation should be extracted from vshCmddefOptParse and performed before any attempt to parse the command's options. Signed-off-by: Erik Skultety--- tools/vsh.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) ACK While this change makes sense on its own, this function is only used for a run-time check of our data. Maybe the only place to call it should be the self test command? Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 4/8] vsh: Extract vshCmddefCheckInternals from vshCmddefOptParse
Originally introduced by commit 2432521e which correctly split vshCmddefOptParse into command's options validation and options parsing. However, command's 'internals' are not tied solely to .options, rather it should be about the overall structure, therefore the validation should be extracted from vshCmddefOptParse and performed before any attempt to parse the command's options. Signed-off-by: Erik Skultety--- tools/vsh.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 4d16299..0626e4f 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -423,9 +423,6 @@ static int vshCmddefOptParse(const vshCmdDef *cmd, uint64_t *opts_need_arg, uint64_t *opts_required) { -if (vshCmddefCheckInternals(cmd) < 0) -return -1; - if (vshCmddefOptFill(cmd, opts_need_arg, opts_required) < 0) return -1; @@ -635,6 +632,12 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) return false; } +if (vshCmddefCheckInternals(def) < 0) { +vshError(ctl, _("internal error: wrong command structure: '%s'"), + def->name); +return false; +} + if (vshCmddefOptParse(def, _need_arg, _required)) { vshError(ctl, _("internal error: bad options in command: '%s'"), def->name); @@ -1410,6 +1413,11 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) vshError(ctl, _("unknown command: '%s'"), tkdata); goto syntaxError; /* ... or ignore this command only? */ } +if (vshCmddefCheckInternals(cmd) < 0) { +vshError(ctl, _("internal error: wrong command structure: " +"'%s'"), tkdata); +goto syntaxError; +} if (vshCmddefOptParse(cmd, _need_arg, _required) < 0) { vshError(ctl, @@ -3344,8 +3352,8 @@ const vshCmdInfo info_selftest[] = { }; /* Prints help for every command. - * That runs vshCmddefOptParse which validates - * the per-command options structure. */ + * That runs vshCmddefCheckInternals which validates + * the overall command structure. */ bool cmdSelfTest(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { -- 2.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list