vl: Avoid to close stdout after finish 'writeconfig' option
PATCH 1/1 vl: Avoid to close stdout after finish 'writeconfig' option
After finish 'writeconfig' to stdout (with '-'), we want to copy/past the related information mannually, not for redirection ('readconfig' does not support '-').
So we can not close the stdout, or next options which may use stdout will not be displayed.
--- vl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
vl.c +3 -1
@@ -3855,7 +3855,9 @@ int main(int argc, char **argv, char **envp) } }qemu_config_write (fp); -fclose (fp); +if (fp != stdout) { +fclose (fp); + }break ; }case QEMU_OPTION_qtest:
Signature · 1 line
1.7.11.7
Quoted 6 lines · Chen Gang
After finish 'writeconfig' to stdout (with '-'), we want to copy/past the related information mannually, not for redirection ('readconfig' does not support '-').
So we can not close the stdout, or next options which may use stdout will not be displayed.
Grammar, and awkward to read. May I suggest:
vl: avoid closing stdout with 'writeconfig'
'writeconfig' supports output to stdout (with '-'); when that happens, we must not close stdout, or further command line options that also use stdout will be impacted. (Although 'writeconfig' was copied from 'readconfig', the latter does not have the problem because it does not support reading from '-')
Quoted 6 lines
Signed-off-by Chen Gang <[email protected]>--- vl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
With the improved commit message,
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 20 lines · Eric Blake
Quoted 6 lines · Chen Gang
After finish 'writeconfig' to stdout (with '-'), we want to copy/past the related information mannually, not for redirection ('readconfig' does not support '-').
So we can not close the stdout, or next options which may use stdout will not be displayed.
Grammar, and awkward to read. May I suggest:
vl: avoid closing stdout with 'writeconfig'
'writeconfig' supports output to stdout (with '-'); when that happens, we must not close stdout, or further command line options that also use stdout will be impacted. (Although 'writeconfig' was copied from 'readconfig', the latter does not have the problem because it does not support reading from '-')
Quoted 2 lines
Quoted 10 lines
Quoted 4 lines
Signed-off-by Chen Gang <[email protected]>--- vl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)With the improved commit message,
Reviewed-by Eric Blake <[email protected]>
OK, it sounds fine to me, I shall send patch v2 for it.
Thanks.
Signature · 3 lines
Chen Gang Open, share, and attitude like air, water, and life which God blessed
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Am 21.04.2014 17:09, schrieb Eric Blake:
Quoted 10 lines · Chen Gang
Quoted 6 lines
After finish 'writeconfig' to stdout (with '-'), we want to copy/past the related information mannually, not for redirection ('readconfig' does not support '-').
So we can not close the stdout, or next options which may use stdout will not be displayed.
Grammar, and awkward to read. May I suggest:
Quoted 7 lines
vl: avoid closing stdout with 'writeconfig'
'writeconfig' supports output to stdout (with '-'); when that happens, we must not close stdout, or further command line options that also use stdout will be impacted. (Although 'writeconfig' was copied from 'readconfig', the latter does not have the problem because it does not support reading from '-')
Eric, did you intentionally request to change "Avoid" to lowercase? I was once pointed to https://wiki.gnome.org/Git/CommitMessages as a template, and starting an English-language heading with an uppercase letter seems natural, with anything else being too lazy to press Shift.
Personally I find -writeconfig the most straightforward way to indicate it's an option, just like () makes clear something is a function. The original subject had an explicit "option", that got lost for v2.
Regards,
Andreas
- --
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBAgAGBQJTVpNxAAoJEPou0S0+fgE/NBEP/RbA40p75QIWuDBM31PXXZNo lrIxfLkb9QVvN7ALUsGOEn6Y1bNQWuItfq5YBN0oDDBxWKSHaU2ou8M3TUWIX6S8 ENLPFJzsVGbzsb4pkP0yMMWZdbs+8AChDJ+oZxgtz6eBf90ERYLarL/t8UjnZos4 wKU3alo2j9xf2OfMpqEHvtxcFc9URDZWKjElPLkZDPn5rFEwcaVGu78rDtjNQIU4 z7Ebc7li29Yppz9MrQ6QoXtU5ZGygPlzZ90G5UZc4v0H0u4ObyYInrWbxqryVp2H k+X173VVpJtPzlhzDkBJPdnwqhoExq7x/n2ujZicRyL95+8ZeoH2kteMRP0eHZYV TDLOkIq2FXklccn91FVVCk6GfLKLdcgZMotaxYD1EIxp7I9NlzpB0d1EVCgClWMx AgZ6StOhYt4+72BaLDOT602FHdsVSEnCqsQeBxoDhKWLwKLklKi59vzDGn+wH0Ap fY33zIW4+YAiXDv3vQg+1k8iG7BauO3cHDVQfbRMzU5zgWORMsm1lsnEv0b3x9YH PhuG9Ve9J2zrbBQEBe0zx98CyB0tp8L/8w5TUVD3EEmQA9YZP5y7I8WU8mOnvgRa Rpu3JF6U3KBO99VF0Yyd3K1mTcG4yJzGfxdLdiTIkSDbEV/jc5aoJFltldZWWpsl DigKmE0VxCFolXJsIsiX
=810z
-----END PGP SIGNATURE-----
Quoted 6 lines · Andreas Färber
Quoted 3 lines
Grammar, and awkward to read. May I suggest:
vl: avoid closing stdout with 'writeconfig'
Eric, did you intentionally request to change "Avoid" to lowercase?
Yes, out of habit, and in comparison to many recent commits. In fact, looking at the current qemu.git head 'git shortlog -100 2d03b49c' shows:
3 commits matching '\.$', 97 without
75 commits matching ': [A-Z]', 24 matching ': [a-z]' (some overlap here, and merge commits don't match either)
extending further to 1000 commits:
21 commits with trailing dot, 979 without
644 matching ': [A-Z]', 260 matching ': [a-z]'
at 10000, things are a bit more even:
5187 matching ': [A-Z]', 3904 matching ': [a-z]'
but still a distinct leaning towards capital at the start of the message
Quoted 3 lines
I was once pointed to https://wiki.gnome.org/Git/CommitMessages as a template, and starting an English-language heading with an uppercase letter seems natural, with anything else being too lazy to press Shift.
Hmm, maybe we should update the qemu wiki?
http://wiki.qemu.org/Contribute/SubmitAPatch doesn't mention the qemu preferred style (you pointed to the GNOME style, but at least GNU coreutils explicitly prefers lowercase).
At any rate, I'm not going to reject a patch based on capitalization, and can retrain my fingers to use a capital if that is the documented preference.
Quoted 4 lines
Personally I find -writeconfig the most straightforward way to indicate it's an option, just like () makes clear something is a function. The original subject had an explicit "option", that got lost for v2.
Good call, so maybe:
vl: Avoid closing stdout with '-writeconfig'
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 6 lines · Eric Blake
Hmm, maybe we should update the qemu wiki?
http://wiki.qemu.org/Contribute/SubmitAPatch doesn't mention the qemu preferred style (you pointed to the GNOME style, but at least GNU coreutils explicitly prefers lowercase).At any rate, I'm not going to reject a patch based on capitalization,
Yeah, it doesn't really seem worth trying to enforce niceties of style here -- the effort required doesn't seem to really match the benefit, and we already have a lot of hoops we make new submitters jump through. "[file or area being patched]: [reasonably descriptive short summary]" is about the limit I personally feel comfortable insisting on.
thanks
-- PMM
[Qemu-trivial] commit log tips [was: [Qemu-devel] [PATCH trivial] vl: Avoid to close stdout after finish 'writeconfig' option]
Quoted 6 lines · Peter Maydell
Quoted 5 lines · Eric Blake
Hmm, maybe we should update the qemu wiki?
http://wiki.qemu.org/Contribute/SubmitAPatch doesn't mention the qemu preferred style (you pointed to the GNOME style, but at least GNU coreutils explicitly prefers lowercase).
Quoted 8 lines
Quoted 1 line
At any rate, I'm not going to reject a patch based on capitalization,
Yeah, it doesn't really seem worth trying to enforce niceties of style here -- the effort required doesn't seem to really match the benefit, and we already have a lot of hoops we make new submitters jump through. "[file or area being patched]: [reasonably descriptive short summary]" is about the limit I personally feel comfortable insisting on.
So, given this discussion, I enhanced the existing paragraph in the wiki page mentioned above to include a couple more sentences:
'''Write a good commit message'''. QEMU follows the usual standard for git commit messages: the first line (which becomes the email subject line) is "subsystem: single line summary of change". Whether the "single line summary of change" starts with a capital is a matter of taste, but we prefer that the summary does not end in ".". Look at <code>git short-log 30</code> for an idea of sample subject lines. Then there is a blank line and a more detailed description of the patch, another blank and your Signed-off-by: line. The body of the commit message is a good place to document why your change is important. Don't include comments like "This is a suggestion for fixing this bug" (they can go below the "---" line in the email so they don't go into the final commit message).
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org