Fried brain today, I have to go through this real slow. I apologize in advance for being denser and more error-prone than usual.
Kevin Wolf <kw...@redhat.com> writes: > This adds a new parameter 'help' to keyval_parse() that enables parsing > of help options. If NULL is passed, the function behaves the same as > before. But if a bool pointer is given, it contains the information > whether an option "help" without value was given (which would otherwise > either result in an error or be interpreted as the value for an implied > key). > > Signed-off-by: Kevin Wolf <kw...@redhat.com> You change the parser, but neglected to update the grammar in the big file comment. I made the mistake of attempting to review the parser change without fixing up the grammar first, and got lost in the weeds. In part because today is not my day, but also because doing parsers without a grammar never works out well for me. Grammar from the big file comment: * KEY=VALUE,... syntax: * * key-vals = [ key-val { ',' key-val } [ ',' ] ] * key-val = key '=' val * key = key-fragment { '.' key-fragment } * key-fragment = / [^=,.]* / * val = { / [^,]* / | ',,' } and * Additional syntax for use with an implied key: * * key-vals-ik = val-no-key [ ',' key-vals ] * val-no-key = / [^=,]* / * * where no-key is syntactic sugar for implied-key=val-no-key. According to the commit message, we want to recognize "help" in place of key-val and val-no-key, but only for callers that opt in. This is more complex than recognizing it in place of just key-vals. The commit message doesn't say why the extra complexity is wanted. Peeking ahead in the series, I can see it's for supporting -object TYPE,help in addition to -object help I see. Making help support opt-in complicates things. Is there a genuine use for not supporting help? Or is just to keep the users that don't support help yet (but should) working without change? Mind, I'm not asking you to make them work, I'm only asking whether you can think of a genuine case where help should not work. What are the existing users that don't support help? Let's see... many in tests/test-keyval.c (ignore), and qobject_input_visitor_new_str(). That one's used for qemu-system-FOO -audiodev, -display, -blockdev, and for qemu-storage-daemon --blockdev, --export, --monitor, --nbd-server. Attempting to get help for them fails like this: $ bld-x86/storage-daemon/qemu-storage-daemon --blockdev help qemu-storage-daemon: Invalid parameter 'help' $ bld-x86/storage-daemon/qemu-storage-daemon --blockdev file,help qemu-storage-daemon: Expected '=' after parameter 'help' I believe making them fail like qemu-storage-daemon: no help available would actually be an improvement. If we get rid of the case "help is not supported", the grammar update isn't too bad, something like * key-val = 'help' | key '=' val and * key-vals-ik = 'help' | val-no-key [ ',' key-vals ] Done for today, hope my brain unfries itself overnight. [...]