* [Qemu-devel] [PATCH] Document Qemu coding style @ 2009-03-29 21:23 Avi Kivity 2009-03-30 1:15 ` malc ` (3 more replies) 0 siblings, 4 replies; 33+ messages in thread From: Avi Kivity @ 2009-03-29 21:23 UTC (permalink / raw) To: Anthony Liguori; +Cc: qemu-devel With the help of some Limoncino I noted several aspects of the Qemu coding style, particularly where it differs from the Linux coding style as many contributors work on both projects. Signed-off-by: Avi Kivity <avi@redhat.com> --- CODING_STYLE | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 77 insertions(+), 0 deletions(-) create mode 100644 CODING_STYLE diff --git a/CODING_STYLE b/CODING_STYLE new file mode 100644 index 0000000..54fdeff --- /dev/null +++ b/CODING_STYLE @@ -0,0 +1,77 @@ +Qemu Coding Style +================= + +1. Whitespace + +Of course, the most important aspect in any coding style is whitespace. +Crusty old coders who have trouble spotting the glasses on their noses +can tell the difference between a tab and eight spaces from a distance +of approximately fifteen parsecs. Many a flamewar have been fought and +lost on this issue. + +Qemu indents are four spaces. Tabs are never used, except in Makefiles +where they have been irreversibly coded into the syntax by some moron. +Spaces of course are superior to tabs because: + + - You have just one way to specify whitespace, not two. Ambiguity breeds + mistakes. + - The confusion surrounding 'use tabs to indent, spaces to justify' is gone. + - Tab indents push your code to the right, making your screen seriously + unbalanced. + - Tabs will be rendered incorrectly on editors who are misconfigured not + to use tab stops of eight positions. + - Tabs are rendered badly in patches, causing off-by-one errors in almost + every line. + - It is the Qemu coding style. + +2. Line width + +Lines are 80 characters wide plus some slop. Try to fit your code into +eighty characters, but if it makes a snippet particularly ugly, allow +yourself some slack. Don't overdo it though. + +Rationale: + - Some people like to tile their 24" screens with a 6x4 matrix of 80x24 + xterms and use vi in all of them. The best way to punish them is to + let them keep doing it. + - Code and especially patches is much more readable if limited to a sane + line length. Eighty is traditional. + - It is the Qemu coding style. + +3. Naming + +Variables are lower_case_with_underscores; easy to type and read. Structured +type names are in CamelCase; harder to type but standing out. Scalar type +names are lower_case_with_underscores_ending_with_a_t, like the Posix +uint64_t and family. + +Typedefs are used to eliminate the redundant 'struct' keyword. It is the +Qemu coding style. + +4. Block structure + +Every indented statement is braced; even if the block contains just one +statement. The opening brace is on the line that contains the control +flow statement that introduces the new block; the closing brace is on the +same line as the else keyword, or on a line by itself if there is no else +keyword. Example: + + if (a == 5) { + printf("a was 5.\n"); + } else if (a == 6) { + printf("a was 6.\n"); + } else { + printf("a was something else entirely.\n"); + } + +An exception is the opening brace for a function; for reasons of tradition +and clarity it comes on a line by itself: + + void a_function(void) + { + do_something(); + } + +Rationale: a consistent (except for functions...) bracing style reduces +ambiguity and avoids needless churn when lines are added or removed. +Furthermore, it is the Qemu coding style. -- 1.6.1.1 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-29 21:23 [Qemu-devel] [PATCH] Document Qemu coding style Avi Kivity @ 2009-03-30 1:15 ` malc 2009-03-30 18:28 ` Blue Swirl ` (2 subsequent siblings) 3 siblings, 0 replies; 33+ messages in thread From: malc @ 2009-03-30 1:15 UTC (permalink / raw) To: qemu-devel On Mon, 30 Mar 2009, Avi Kivity wrote: > With the help of some Limoncino I noted several aspects of the Qemu coding QEMU > style, particularly where it differs from the Linux coding style as many > contributors work on both projects. > > Signed-off-by: Avi Kivity <avi@redhat.com> > --- > CODING_STYLE | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 77 insertions(+), 0 deletions(-) > create mode 100644 CODING_STYLE > > diff --git a/CODING_STYLE b/CODING_STYLE > new file mode 100644 > index 0000000..54fdeff > --- /dev/null > +++ b/CODING_STYLE > @@ -0,0 +1,77 @@ > +Qemu Coding Style > +================= > + > +1. Whitespace > + > +Of course, the most important aspect in any coding style is whitespace. > +Crusty old coders who have trouble spotting the glasses on their noses > +can tell the difference between a tab and eight spaces from a distance > +of approximately fifteen parsecs. Many a flamewar have been fought and > +lost on this issue. > + > +Qemu indents are four spaces. Tabs are never used, except in Makefiles > +where they have been irreversibly coded into the syntax by some moron. > +Spaces of course are superior to tabs because: > + > + - You have just one way to specify whitespace, not two. Ambiguity breeds > + mistakes. > + - The confusion surrounding 'use tabs to indent, spaces to justify' is gone. > + - Tab indents push your code to the right, making your screen seriously > + unbalanced. > + - Tabs will be rendered incorrectly on editors who are misconfigured not > + to use tab stops of eight positions. > + - Tabs are rendered badly in patches, causing off-by-one errors in almost > + every line. > + - It is the Qemu coding style. > + > +2. Line width > + > +Lines are 80 characters wide plus some slop. Try to fit your code into > +eighty characters, but if it makes a snippet particularly ugly, allow > +yourself some slack. Don't overdo it though. > + > +Rationale: > + - Some people like to tile their 24" screens with a 6x4 matrix of 80x24 > + xterms and use vi in all of them. The best way to punish them is to > + let them keep doing it. > + - Code and especially patches is much more readable if limited to a sane > + line length. Eighty is traditional. > + - It is the Qemu coding style. > + > +3. Naming > + > +Variables are lower_case_with_underscores; easy to type and read. Structured > +type names are in CamelCase; harder to type but standing out. Scalar type > +names are lower_case_with_underscores_ending_with_a_t, like the Posix > +uint64_t and family. POSIX anything_than_ends_with_a_t is reserved (http://www.opengroup.org/pubs/online/7908799/xns/namespace.html) QEMU does use quite a few of types ending with a _t and this should be fixed. > + > +Typedefs are used to eliminate the redundant 'struct' keyword. It is the > +Qemu coding style. > + > +4. Block structure > + > +Every indented statement is braced; even if the block contains just one > +statement. The opening brace is on the line that contains the control > +flow statement that introduces the new block; the closing brace is on the > +same line as the else keyword, or on a line by itself if there is no else > +keyword. Example: > + > + if (a == 5) { > + printf("a was 5.\n"); > + } else if (a == 6) { > + printf("a was 6.\n"); > + } else { > + printf("a was something else entirely.\n"); > + } > + > +An exception is the opening brace for a function; for reasons of tradition > +and clarity it comes on a line by itself: > + > + void a_function(void) > + { > + do_something(); > + } > + > +Rationale: a consistent (except for functions...) bracing style reduces > +ambiguity and avoids needless churn when lines are added or removed. > +Furthermore, it is the Qemu coding style. > -- mailto:av1474@comtv.ru ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-29 21:23 [Qemu-devel] [PATCH] Document Qemu coding style Avi Kivity 2009-03-30 1:15 ` malc @ 2009-03-30 18:28 ` Blue Swirl 2009-03-30 19:02 ` M. Warner Losh ` (4 more replies) 2009-03-31 13:47 ` Paul Brook 2009-04-01 8:51 ` Richard W.M. Jones 3 siblings, 5 replies; 33+ messages in thread From: Blue Swirl @ 2009-03-30 18:28 UTC (permalink / raw) To: qemu-devel On 3/30/09, Avi Kivity <avi@redhat.com> wrote: > With the help of some Limoncino I noted several aspects of the Qemu coding > style, particularly where it differs from the Linux coding style as many > contributors work on both projects. Ok, I'm armed with some Foster's. > > Signed-off-by: Avi Kivity <avi@redhat.com> > --- > CODING_STYLE | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 77 insertions(+), 0 deletions(-) > create mode 100644 CODING_STYLE > > diff --git a/CODING_STYLE b/CODING_STYLE > new file mode 100644 > index 0000000..54fdeff > --- /dev/null > +++ b/CODING_STYLE > @@ -0,0 +1,77 @@ > +Qemu Coding Style > +================= > + > +1. Whitespace > + > +Of course, the most important aspect in any coding style is whitespace. > +Crusty old coders who have trouble spotting the glasses on their noses > +can tell the difference between a tab and eight spaces from a distance > +of approximately fifteen parsecs. Many a flamewar have been fought and > +lost on this issue. > + > +Qemu indents are four spaces. Tabs are never used, except in Makefiles > +where they have been irreversibly coded into the syntax by some moron. > +Spaces of course are superior to tabs because: > + > + - You have just one way to specify whitespace, not two. Ambiguity breeds > + mistakes. > + - The confusion surrounding 'use tabs to indent, spaces to justify' is gone. > + - Tab indents push your code to the right, making your screen seriously > + unbalanced. > + - Tabs will be rendered incorrectly on editors who are misconfigured not > + to use tab stops of eight positions. > + - Tabs are rendered badly in patches, causing off-by-one errors in almost > + every line. > + - It is the Qemu coding style. Never leave whitespace at the end of line, it annoys the Quilt. Empty lines at the end of file just make files bigger. > + > +2. Line width > + > +Lines are 80 characters wide plus some slop. Try to fit your code into > +eighty characters, but if it makes a snippet particularly ugly, allow > +yourself some slack. Don't overdo it though. > + > +Rationale: > + - Some people like to tile their 24" screens with a 6x4 matrix of 80x24 > + xterms and use vi in all of them. The best way to punish them is to > + let them keep doing it. > + - Code and especially patches is much more readable if limited to a sane > + line length. Eighty is traditional. > + - It is the Qemu coding style. > + > +3. Naming > + > +Variables are lower_case_with_underscores; easy to type and read. Structured > +type names are in CamelCase; harder to type but standing out. Scalar type > +names are lower_case_with_underscores_ending_with_a_t, like the Posix > +uint64_t and family. > + > +Typedefs are used to eliminate the redundant 'struct' keyword. It is the > +Qemu coding style. > + > +4. Block structure > + > +Every indented statement is braced; even if the block contains just one > +statement. I'd remove this, braces are not used consistently for one statement blocks. > The opening brace is on the line that contains the control > +flow statement that introduces the new block; the closing brace is on the > +same line as the else keyword, or on a line by itself if there is no else > +keyword. Example: > + > + if (a == 5) { > + printf("a was 5.\n"); > + } else if (a == 6) { > + printf("a was 6.\n"); > + } else { > + printf("a was something else entirely.\n"); > + } > + > +An exception is the opening brace for a function; for reasons of tradition > +and clarity it comes on a line by itself: > + > + void a_function(void) > + { > + do_something(); > + } > + > +Rationale: a consistent (except for functions...) bracing style reduces > +ambiguity and avoids needless churn when lines are added or removed. > +Furthermore, it is the Qemu coding style. No, this is the K&R style. Quoting linux/Documentation/CodingStyle: Heretic people all over the world have claimed that this inconsistency is ... well ... inconsistent, but all right-thinking people know that (a) K&R are _right_ and (b) K&R are right. Besides, functions are special anyway (you can't nest them in C). Cheers! ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 18:28 ` Blue Swirl @ 2009-03-30 19:02 ` M. Warner Losh 2009-03-30 19:55 ` Avi Kivity 2009-03-30 19:54 ` Avi Kivity ` (3 subsequent siblings) 4 siblings, 1 reply; 33+ messages in thread From: M. Warner Losh @ 2009-03-30 19:02 UTC (permalink / raw) To: qemu-devel, blauwirbel In message: <f43fc5580903301128y2a8432f4u6b7d9375e29a5c82@mail.gmail.com> Blue Swirl <blauwirbel@gmail.com> writes: : > +An exception is the opening brace for a function; for reasons of tradition : > +and clarity it comes on a line by itself: : > + : > + void a_function(void) : > + { : > + do_something(); : > + } : > + : > +Rationale: a consistent (except for functions...) bracing style reduces : > +ambiguity and avoids needless churn when lines are added or removed. : > +Furthermore, it is the Qemu coding style. : : No, this is the K&R style. Quoting linux/Documentation/CodingStyle: : : Heretic people all over the world have claimed that this inconsistency : is ... well ... inconsistent, but all right-thinking people know that : (a) K&R are _right_ and (b) K&R are right. Besides, functions are : special anyway (you can't nest them in C). And besides, there's lots of almost-smart tools that operate on source code that know this is always true... Or at least historically that's been the case, don't know of any widely used ones today, but I doubt they were all fixed... Warner ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 19:02 ` M. Warner Losh @ 2009-03-30 19:55 ` Avi Kivity 0 siblings, 0 replies; 33+ messages in thread From: Avi Kivity @ 2009-03-30 19:55 UTC (permalink / raw) To: qemu-devel; +Cc: blauwirbel M. Warner Losh wrote: > In message: <f43fc5580903301128y2a8432f4u6b7d9375e29a5c82@mail.gmail.com> > Blue Swirl <blauwirbel@gmail.com> writes: > : > +An exception is the opening brace for a function; for reasons of tradition > : > +and clarity it comes on a line by itself: > : > + > : > + void a_function(void) > : > + { > : > + do_something(); > : > + } > : > + > : > +Rationale: a consistent (except for functions...) bracing style reduces > : > +ambiguity and avoids needless churn when lines are added or removed. > : > +Furthermore, it is the Qemu coding style. > : > : No, this is the K&R style. Quoting linux/Documentation/CodingStyle: > : > : Heretic people all over the world have claimed that this inconsistency > : is ... well ... inconsistent, but all right-thinking people know that > : (a) K&R are _right_ and (b) K&R are right. Besides, functions are > : special anyway (you can't nest them in C). > > And besides, there's lots of almost-smart tools that operate on source > code that know this is always true... Or at least historically that's > been the case, don't know of any widely used ones today, but I doubt > they were all fixed... > Relax, no one wants to change this... -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 18:28 ` Blue Swirl 2009-03-30 19:02 ` M. Warner Losh @ 2009-03-30 19:54 ` Avi Kivity 2009-03-30 21:43 ` Lennart Sorensen 2009-03-30 19:58 ` Avi Kivity ` (2 subsequent siblings) 4 siblings, 1 reply; 33+ messages in thread From: Avi Kivity @ 2009-03-30 19:54 UTC (permalink / raw) To: qemu-devel Blue Swirl wrote: >> +4. Block structure >> + >> +Every indented statement is braced; even if the block contains just one >> +statement. >> > > I'd remove this, braces are not used consistently for one statement blocks. > > While that's true, I'd like to keep this. I found (after initially being annoyed by this; I dislike punctuation) that it's nice not to need to rebrace after adding or removing lines. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 19:54 ` Avi Kivity @ 2009-03-30 21:43 ` Lennart Sorensen 2009-03-30 22:15 ` M. Warner Losh 0 siblings, 1 reply; 33+ messages in thread From: Lennart Sorensen @ 2009-03-30 21:43 UTC (permalink / raw) To: qemu-devel On Mon, Mar 30, 2009 at 10:54:58PM +0300, Avi Kivity wrote: > Blue Swirl wrote: >>> +4. Block structure >>> + >>> +Every indented statement is braced; even if the block contains just one >>> +statement. >>> >> >> I'd remove this, braces are not used consistently for one statement blocks. >> >> > > While that's true, I'd like to keep this. I found (after initially > being annoyed by this; I dislike punctuation) that it's nice not to need > to rebrace after adding or removing lines. I hate having to add braces to add a printf when debuging something. If you forget the braces, you change the meaning of the code. To me that's a disaster and enough reason that leaving out braces should be banned. Code style that encourages creation of bugs is bad style. Surprisingly many code styles don't get this however. I also like consistency, and since you need braces for multiline statements, why not always use them? -- Len Sorensen ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 21:43 ` Lennart Sorensen @ 2009-03-30 22:15 ` M. Warner Losh 2009-03-30 23:38 ` Lennart Sorensen 0 siblings, 1 reply; 33+ messages in thread From: M. Warner Losh @ 2009-03-30 22:15 UTC (permalink / raw) To: qemu-devel, lsorense In message: <20090330214321.GP3795@csclub.uwaterloo.ca> lsorense@csclub.uwaterloo.ca (Lennart Sorensen) writes: : On Mon, Mar 30, 2009 at 10:54:58PM +0300, Avi Kivity wrote: : > Blue Swirl wrote: : >>> +4. Block structure : >>> + : >>> +Every indented statement is braced; even if the block contains just one : >>> +statement. : >>> : >> : >> I'd remove this, braces are not used consistently for one statement blocks. : >> : >> : > : > While that's true, I'd like to keep this. I found (after initially : > being annoyed by this; I dislike punctuation) that it's nice not to need : > to rebrace after adding or removing lines. : : I hate having to add braces to add a printf when debuging something. : If you forget the braces, you change the meaning of the code. To me : that's a disaster and enough reason that leaving out braces should : be banned. Code style that encourages creation of bugs is bad style. : Surprisingly many code styles don't get this however. With editors like emacs, this isn't an issue. : I also like consistency, and since you need braces for multiline : statements, why not always use them? Because it stretches the code vertically. More extra useless 'blank' lines makes it harder to get more code on the screen, which makes the code harder to understand. Anyway, this is a highly religious issue. Either you think that {} are the bee's knees and people are morons that don't use them, or you hate them with a huge passion and can't believe people are stupid enough to require it. There's a very small set of folks in between, and often little common ground: usually one camp tolerates the practices of the other... Warner ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 22:15 ` M. Warner Losh @ 2009-03-30 23:38 ` Lennart Sorensen 2009-03-31 0:09 ` M. Warner Losh 2009-03-31 5:59 ` Laurent Desnogues 0 siblings, 2 replies; 33+ messages in thread From: Lennart Sorensen @ 2009-03-30 23:38 UTC (permalink / raw) To: qemu-devel On Mon, Mar 30, 2009 at 04:15:14PM -0600, M. Warner Losh wrote: > With editors like emacs, this isn't an issue. Who gives a @#$ what emacs does. > Because it stretches the code vertically. More extra useless 'blank' > lines makes it harder to get more code on the screen, which makes the > code harder to understand. So what? Compared to the bugs this often causes, go buy a bigger screen. > Anyway, this is a highly religious issue. Either you think that {} > are the bee's knees and people are morons that don't use them, or you > hate them with a huge passion and can't believe people are stupid > enough to require it. There's a very small set of folks in between, > and often little common ground: usually one camp tolerates the > practices of the other... I just hate the mistakes the lack of the braces cause, and they do cause mistakes. It is a huge mistake that C even allowed them to be optional in the first place. A bit late to fix that now. -- Len Sorensen ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 23:38 ` Lennart Sorensen @ 2009-03-31 0:09 ` M. Warner Losh 2009-03-31 5:59 ` Laurent Desnogues 1 sibling, 0 replies; 33+ messages in thread From: M. Warner Losh @ 2009-03-31 0:09 UTC (permalink / raw) To: qemu-devel, lsorense In message: <20090330233853.GT3795@csclub.uwaterloo.ca> lsorense@csclub.uwaterloo.ca (Lennart Sorensen) writes: : On Mon, Mar 30, 2009 at 04:15:14PM -0600, M. Warner Losh wrote: : > With editors like emacs, this isn't an issue. : : Who gives a @#$ what emacs does. : : > Because it stretches the code vertically. More extra useless 'blank' : > lines makes it harder to get more code on the screen, which makes the : > code harder to understand. : : So what? Compared to the bugs this often causes, go buy a bigger screen. When used with emacs, the number of bugs introduced is about nil, while the bugs introduced through lack of understanding of the code is non-nil. At least that's been my experience over the past 20 years of doing this. Your mileage may vary. Objects in mirror are closer than they appear. : > Anyway, this is a highly religious issue. Either you think that {} : > are the bee's knees and people are morons that don't use them, or you : > hate them with a huge passion and can't believe people are stupid : > enough to require it. There's a very small set of folks in between, : > and often little common ground: usually one camp tolerates the : > practices of the other... : : I just hate the mistakes the lack of the braces cause, and they do : cause mistakes. It is a huge mistake that C even allowed them to be : optional in the first place. A bit late to fix that now. Again, this is clearly a religious argument. Nobody is going to settle it here. Warner ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 23:38 ` Lennart Sorensen 2009-03-31 0:09 ` M. Warner Losh @ 2009-03-31 5:59 ` Laurent Desnogues 2009-03-31 12:58 ` David Turner 1 sibling, 1 reply; 33+ messages in thread From: Laurent Desnogues @ 2009-03-31 5:59 UTC (permalink / raw) To: qemu-devel On Tue, Mar 31, 2009 at 12:38 AM, Lennart Sorensen <lsorense@csclub.uwaterloo.ca> wrote: > On Mon, Mar 30, 2009 at 04:15:14PM -0600, M. Warner Losh wrote: >> With editors like emacs, this isn't an issue. > > Who gives a @#$ what emacs does. Ha at last! vi was put in the original document, then someone mentions emacs and then some insults. This finally becomes interesting. Laurent ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 5:59 ` Laurent Desnogues @ 2009-03-31 12:58 ` David Turner 2009-03-31 13:31 ` Avi Kivity ` (2 more replies) 0 siblings, 3 replies; 33+ messages in thread From: David Turner @ 2009-03-31 12:58 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 1259 bytes --] Very frankly, I don't think that a coding style, even strictly applied, is going to make the QEMU code easier to understand. The real barriers to understanding are the lack of structure in the code, liberal use of global macros scattered randomly in the source code, exceedingly liberally named functions, and sometimes obscure implementation of simple concepts (*cough* CharDriverState), cramming totally unrelated stuff in single largish source files (vl.c for the win !), and a blatant lack of documentation comments for a lot of subtle stuff in there to explain the magic. Braces and indentation are sometimes annoying, but frankly these are such minor issues I wonder why people waste their time venting about them given the source code's state. On Tue, Mar 31, 2009 at 7:59 AM, Laurent Desnogues < laurent.desnogues@gmail.com> wrote: > On Tue, Mar 31, 2009 at 12:38 AM, Lennart Sorensen > <lsorense@csclub.uwaterloo.ca> wrote: > > On Mon, Mar 30, 2009 at 04:15:14PM -0600, M. Warner Losh wrote: > >> With editors like emacs, this isn't an issue. > > > > Who gives a @#$ what emacs does. > > Ha at last! vi was put in the original document, then someone mentions > emacs and then some insults. This finally becomes interesting. > > > Laurent > > > [-- Attachment #2: Type: text/html, Size: 1740 bytes --] ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 12:58 ` David Turner @ 2009-03-31 13:31 ` Avi Kivity 2009-03-31 21:18 ` David Turner 2009-03-31 16:18 ` Blue Swirl 2009-04-01 9:04 ` Daniel P. Berrange 2 siblings, 1 reply; 33+ messages in thread From: Avi Kivity @ 2009-03-31 13:31 UTC (permalink / raw) To: qemu-devel David Turner wrote: > Very frankly, I don't think that a coding style, even strictly > applied, is going to make the QEMU code > easier to understand. > The coding style is not intended to make the code clearer, just to make it more uniform. > The real barriers to understanding are the lack of structure in the > code, liberal use of global macros > scattered randomly in the source code, exceedingly liberally named > functions, and sometimes obscure > implementation of simple concepts (*cough* CharDriverState), cramming > totally unrelated stuff in single > largish source files (vl.c for the win !), and a blatant lack of > documentation comments for a lot of subtle > stuff in there to explain the magic. This is the QEMU coding style. Seriously, what you say is largely true, but the way to fix it is to sent patches or to review posted patches. Nothing else will make a difference. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 13:31 ` Avi Kivity @ 2009-03-31 21:18 ` David Turner 0 siblings, 0 replies; 33+ messages in thread From: David Turner @ 2009-03-31 21:18 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 245 bytes --] On Tue, Mar 31, 2009 at 3:31 PM, Avi Kivity <avi@redhat.com> wrote: > The coding style is not intended to make the code clearer, just to make it > more uniform. > I'm just saying that uniformity is overrated given the state of the sources :-) [-- Attachment #2: Type: text/html, Size: 543 bytes --] ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 12:58 ` David Turner 2009-03-31 13:31 ` Avi Kivity @ 2009-03-31 16:18 ` Blue Swirl 2009-03-31 21:48 ` David Turner 2009-04-01 9:04 ` Daniel P. Berrange 2 siblings, 1 reply; 33+ messages in thread From: Blue Swirl @ 2009-03-31 16:18 UTC (permalink / raw) To: qemu-devel On 3/31/09, David Turner <digit@google.com> wrote: > Very frankly, I don't think that a coding style, even strictly applied, is > going to make the QEMU code > easier to understand. > > The real barriers to understanding are the lack of structure in the code, > liberal use of global macros > scattered randomly in the source code, exceedingly liberally named > functions, and sometimes obscure > implementation of simple concepts (*cough* CharDriverState), cramming > totally unrelated stuff in single > largish source files (vl.c for the win !), and a blatant lack of > documentation comments for a lot of subtle > stuff in there to explain the magic. True enough for the comments part. There are still some areas that I don't understand well, for example TB handling and its inherent limitations. Which part of the source you find subtle and magical but not commented enough? Maybe we should use Doxygen or hxtool to combine source code and documentation. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 16:18 ` Blue Swirl @ 2009-03-31 21:48 ` David Turner 2009-03-31 22:38 ` malc 0 siblings, 1 reply; 33+ messages in thread From: David Turner @ 2009-03-31 21:48 UTC (permalink / raw) To: qemu-devel [-- Attachment #1.1: Type: text/plain, Size: 2914 bytes --] On Tue, Mar 31, 2009 at 6:18 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > > True enough for the comments part. There are still some areas that I > don't understand well, for example TB handling and its inherent > limitations. > > Which part of the source you find subtle and magical but not commented > enough? > ok, here are a few things that ring a bell, there are probably a lot others: the software mmu implementation in system mode is *really* hard to understand at first. It took me a long time to grasp the various aspects of it, including these: - loads/stores in kernel or userspace map to different translated code fragments - loads/stores in different emulated CPUs also map to different translated code. - the way i/o memory access is controlled in fine details and relates to the rest of the MMU. most of what happens in exe.c is black-magic :-) how the audio-subsystem works and its relationship with hardware emulation is subtle. For my work on the Android emulator, I have modified three audio-backends and wrote one from scratch. It took me several tries to get things to an acceptable state. What I didn't understand first is that there is no common time-based used by all components involved. dyngen used to be pretty radical too. Thanks god it is gone for any target and host platform combination I care about :-) slirp is an hideous pile of goo which mixes network-ordered fields and host-ordered ones, including pointers, into the same structures, liberally and at different times, depending on context. Which is probably why it took so long to fix 64-bit issues in it. I would like to add support for IPv6 to this code, but sadly, I have the feeling that rewriting most of it from scratch would be slightly easier. the CharDriverState interface takes some time to fully understand. In the Android emulator, I sometimes need to connect two CharDriverState users together, and had to write a special purpose object just to do that, but was surprised how hard writing a bug-less one was. I also think that the event loop implementation is confusing compared to more common interfaces provides by things like libevent. It is also extremely tied to select(), which prevents using better mechanisms on various platforms, and even performs rather poorly on Windows, but I digress. qemu_get_clock(vm_clock) will return time in nano-seconds, but qemu_get_clock(rt_clock) will return time in milli-seconds. This is totally undocumented, and the code that uses the result tend to use magic numbers like 1000000 or 1000 to perform conversions which are never clear on first sight. Maybe this is fixed in upstream QEMU but, my, how this hurted me in the past. For the record, here are attached a few documents I wrote to detail what I've "discovered" until now. Hope some one can find them useful (Sorry if some of them are focused on ARM system emulation only). Regards [-- Attachment #1.2: Type: text/html, Size: 3409 bytes --] [-- Attachment #2: AUDIO.TXT --] [-- Type: text/plain, Size: 8296 bytes --] HOW AUDIO EMULATION WORKS IN QEMU: ================================== Things are a bit tricky, but here's a rough description: QEMUSoundCard: models a given emulated sound card SWVoiceOut: models an audio output from a QEMUSoundCard SWVoiceIn: models an audio input from a QEMUSoundCard HWVoiceOut: models an audio output (backend) on the host. HWVoiceIn: models an audio input (backend) on the host. Each voice can have its own settings in terms of sample size, endianess, rate, etc... Emulation for a given soundcard typically does: 1/ Create a QEMUSoundCard object and register it with AUD_register_card() 2/ For each emulated output, call AUD_open_out() to create a SWVoiceOut object. 3/ For each emulated input, call AUD_open_in() to create a SWVoiceIn object. Note that you must pass a callback function to AUD_open_out() and AUD_open_in(); more on this later. Each SWVoiceOut is associated to a single HWVoiceOut, each SWVoiceIn is associated to a single HWVoiceIn. However you can have several SWVoiceOut associated to the same HWVoiceOut (same thing for SWVoiceIn/HWVoiceIn). SOUND PLAYBACK DETAILS: ======================= Each HWVoiceOut has the following too: - A fixed-size circular buffer of stereo samples (for stereo). whose format is either floats or int64_t per sample (depending on build configuration). - A 'samples' field giving the (constant) number of sample pairs in the stereo buffer. - A target conversion function, called 'clip()' that is used to read from the stereo buffer and write into a platform-specific sound buffers (e.g. WinWave-managed buffers on Windows). - A 'rpos' offset into the circular buffer which tells where to read the next samples from the stereo buffer for the next conversion through 'clip'. |<----------------- samples ----------------------->| | | | rpos | | |_______v___________________________________________| | | | | | | |_______|___________________________________________| - A 'run_out' method that is called each time to tell the output backend to send samples from the stereo buffer to the host sound card/server. This method shall also modify 'rpos' and returns the number of samples 'played'. A more detailed description of this process appears below. - A 'write' method callback used to write a buffer of emulated sound samples from a SWVoiceOut into the stereo buffer. *All* backends simply call the generic function audio_pcm_sw_write() to implement this. It's difficult to see why it's needed at all ? (Similarly, all backends have a 'read' methods which simply calls 'audio_pcm_sw_read') Each SWVoiceOut has the following: - a 'conv()' function used to read sound samples from the emulated sound card and copy/mix them to the corresponding HWVoiceOut's stereo buffer. - a 'total_hw_samples_mixed' which correspond to the number of samples that have already been mixed into the target HWVoiceOut stereo buffer (starting from the HWVoiceOut's 'rpos' offset). NOTE: this is a count of samples in the HWVoiceOut stereo buffer, not emulated hardware sound samples, which can have different properties (frequency, size, endianess). ______________ | | | SWVoiceOut2 | |______________| ______________ | | | | | SWVoiceOut1 | | thsm<N> := total_hw_samples_mixed |______________| | for SWVoiceOut<N> | | | | |<-----|------------thsm2-->| | | | |<---thsm1-------->| | _______|__________________v________|_______________ | |111111111111111111| v | | |222222222222222222222222222| | |_______|___________________________________________| ^ | HWVoiceOut stereo buffer rpos - a 'ratio' value, which is the ratio of the target HWVoiceOut's frequency by the SWVoiceOut's frequency, multiplied by (1 << 32), as a 64-bit integer. So, if the HWVoiceOut has a frequency of 44kHz, and the SWVoiceOut has a frequency of 11kHz, then ratio will be (44/11*(1 << 32)) = 0x4_0000_0000 - a callback provided by the emulated hardware when the SWVoiceOut is created. This function is used to mix the SWVoiceOut's samples into the target HWVoiceOut stereo buffer (it must also perform frequency interpolation, volume adjustment, etc..). This callback normally calls another helper functions in the audio subsystem (AUD_write()) to to the mixing/volume-adjustment from emulated hardware sample buffers. Here's a small graphics that explains it better: SWVoiceOut: emulated hardware sound buffers: | | (mixed through AUD_write() from user-provided callback | which is called on each audio timer tick). v HWVoiceOut: stereo sample circular buffer | | (through HWVoiceOut's 'clip' function, invoked from the | 'run_out' method) v backend-specific sound buffers THERE IS NO COMMON TIMEBASE BETWEEN ALL LAYERS. DON'T EXPECT ANY HIGH-ACCURACY / LOW-LATENCY IN THIS IMPLEMENTATION. The function audio_timer() in audio/audio.c is called periodically and it is used as a pulse to perform sound buffer transfers and mixing. More specifically for audio output voices: - For each HWVoiceOut, find the number of active SWVoiceOut, and the minimum number of 'total_hw_samples_mixed' that have already been written to the buffer. We will call this value the number of 'live' samples in the stereo buffer. - if 'live' is 0, call the callback of each active SWVoiceOut to fill the stereo buffer, if needed, then exit. - otherwise, call the 'run_out' method of the HWVoiceOut object. This will change the value of 'rpos' and return the number of samples played. Then the 'total_hw_samples_mixed' field of all active SWVoiceOuts is decremented by 'played', and the callback is called to re-fill the stereo buffer. It's important to note that the SWVoiceOut callback: - takes a 'free' parameter which is the number of stereo sound samples that can be sent to the hardware stereo buffer (before rate adjustment, i.e. not the number of sound samples in the SWVoiceOut emulated hardware sound buffer). - must call AUD_write(sw, buff, count), where 'buff' points to emulated sound samples, and their 'count', which must be <= the 'free' parameter. - the implementation of AUD_write() will call the 'write' method of the target HWVoiceOut, which in turns calls the function audio_pcm_sw_write() which does standard rate/volume adjustment before mixing the conversion into the target stereo buffer. It also increases the 'total_hw_samples_mixed' value of the SWVoiceOut. - audio_pcm_sw_write() returns the number of sound sample *bytes* that have been mixed into the stereo buffer, and so does AUD_write(). So, in the end, we have the pseudo-code: every sound timer ticks: for hw in list_HWVoiceOut: live = MIN([sw.total_hw_samples_mixed for sw in hw.list_SWVoiceOut ]) if live > 0: played = hw.run_out(live) for sw in hw.list_SWVoiceOut: sw.total_hw_samples_mixed -= played for sw in hw.list_SWVoiceOut: free = hw.samples - sw.total_hw_samples_mixed if free > 0: sw.callback(sw, free) SOUND RECORDING DETAILS: ======================== Things are similar but in reverse order. [-- Attachment #3: CHAR-DEVICES.TXT --] [-- Type: text/plain, Size: 8264 bytes --] QEMU CHARACTER "DEVICES" MANAGEMENT I. CharDriverState objects: --------------------------- One of the strangest abstraction in QEMU is the "CharDriverState" (abbreviated here as "CS"). The CS is essentially an object used to model a character stream that can be connected to things like a host serial port, a host network socket, an emulated device, etc... What's really unusual is its interface though, which comes from the fact that QEMU implements a big event loop with no blocking i/o allowed. You can see "qemu-char.h" for the full interface, but here we will only describe a few important functions: - qemu_chr_write() is used to try to write data into a CS object. Note that success is not guaranteed: the function returns the number of bytes that were really written (which can be 0) and the caller must deal with it. This is very similar to writing to a non-blocking BSD socket on Unix. int qemu_chr_read( CharDriverState* cs, const uint8_t* data, int datalen ); This function may return -1 in case of error, but this depends entirely on the underlying implementation (some of them will just return 0 instead). In practice, this means it's not possible to reliably differentiate between a "connection reset by peer" and an "operation in progress" :-( There is no way to know in advance how many bytes a given CharDriverState can accept, nor to be notified when its underlying implementation is ready to accept data again. - qemu_chr_add_handler() is used to add "read" and "event" handlers to a CS object. We will ignore "events" here and focus on the "read" part. Thing is, you cannot directly read from a CS object. Instead, you provide two functions that will be called whenever the object has something for you: - a 'can_read' function that shall return the number of bytes that you are ready to accept from the CharDriverState. It's interface is: typedef int IOCanRWHandler (void* opaque); - a 'read' function that will send you bytes from the CharDriverState typedef void IOReadHandler (void* opaque, const uint8_t* data, int datalen); normally, the value of 'datalen' cannot be larger than the result of a previous 'can_read' call. For both callbacks, 'opaque' is a value that you pass to the function qemu_chr_add_handler() which signature is: void qemu_chr_add_handlers(CharDriverState *s, IOCanRWHandler *fd_can_read, IOReadHandler *fd_read, IOEventHandler *fd_event, void *opaque); - qemu_chr_open() is used to create a new CharDriverState object from a descriptive string, it's interface is: CharDriverState* qemu_chr_open(const char* filename); there are various formats for acceptable 'filenames', and they correspond to the parameters of the '-serial' QEMU option described here: http://www.nongnu.org/qemu/qemu-doc.html#SEC10 For example: "/dev/<file>" (Linux and OS X only): connect to a host character device file (e.g. /dev/ttyS0) "file:<filename>": Write output to a given file (write only) "stdio": Standard input/output "udp:[<remote_host>]:<remote_port>[@[<src_ip>]:<src_port>]": Connect to a UDP socket for both read/write. "tcp:[<host>]:<port>[,server][,nowait][,nodelay]" Connect to a TCP socket either as a client or a server. The 'nowait' option is used to avoid waiting for a client connection. The 'nodelay' is used to disable the TCP Nagle algorithm to improve throughput. for Android, a few special names have been added to the internal implementation and redirect to program functions: "android-kmsg": A CharDriverState that is used to receive kernel log messages from the emulated /dev/ttyS0 serial port. "android-qemud": A CharDriverState that is used to exchange messages between the emulator program and the "qemud" multiplexing daemon that runs in the emulated system. The "qemud" daemon is used to allow one or more clients in the system to connect to various services running in the emulator program. This is mainly used to bypass the kernel in order to implement certain features with ease. "android-gsm": A CharDriverState that is used to connect the emulated system to a host modem device with the -radio <device> option. Otherwise, the system uses qemud to connect to the emulator's internal modem emulation. "android-gps": A CharDriverState that is used to connect the emulated system to a host GPS device with the -gps <device> option. Otherwise the system uses qemud to connect to the emulator's internal GPS emulation. II. CharDriverState users: -------------------------- As described above, a CharDriverState "user" is a piece of code that can write to a CharDriverState (by calling qemu_chr_write() explicitely) and can also read from it after registering can_read/read handlers for it through qemu_chr_add_handlers(). Typical examples are the following: - The hardware serial port emulation (e.g. hw/goldfish_tty.c) will read data from the kernel then send it to a CS. It also uses a small buffer that is used to read data from the CS and send it back to the kernel. - The Android emulated modem also uses a CS to talk with its client, which will in most cases an emulated serial port. III. CharBuffer objects: ------------------------ The Android emulator provides an object called a CharBuffer which acts as a CharDriverState object that implements a *write* buffer to send data to a given CS object, called the endpoint. You can create one with: #include "charpipe.h" CharDriverState* qemu_chr_open_buffer( CharDriverState* endpoint ); This function returns a new CS object that will buffer in the heap any data that is sent to it, but cannot be sent to the endpoint yet. On each event loop iteration, the CharBuffer will try to send data to the endpoint untill it doesn't have any data left. This can be useful to simplify certain CS users who don't want to maintain their own emit buffer. Note that writing to a CharBuffer always succeeds. Note also that calling qemu_chr_add_handler() on the CharBuffer will do the same on the endpoint. Any endpoint-initiated calls to can_read()/read() callbacks are passed directly to your handler functions. IV. CharPipe objects: --------------------- The Android emulator also provides a convenient abstraction called a "charpipe" used to connect two CharDriverState users together. For example, this is used to connect a serial port emulation (in hw/goldfish_tty.c) to the internal GSM modem emulation (see telephony/modem_driver.c). Essentially, a "charpipe" is a bi-directionnal communication pipe whose two endpoints are both CS objects. You call "qemu_chr_open_pipe()" to create the pipe, and this function will return the two endpoints to you: #include "charpipe.h" int qemu_chr_open_pipe(CharDriverState* *pfirst, CharDriverState* *psecond); When you write to one end of the pipe (with qemu_chr_write()), the charpipe will try to write as much data as possible to the other end. Any remaining data is stored in a heap-allocated buffer. The charpipe will try to re-send the buffered data on the next event loop iteration by calling the can_read/read functions of the corresponding user, if there is one. Note that there is no limit on the amount of data buffered in a charpipe, and writing to it is never blocking. This simplifies CharDriverState users who don't need to worry about buffering issues. [-- Attachment #4: CPU-EMULATION.TXT --] [-- Type: text/plain, Size: 3949 bytes --] HOW THE QEMU EXECUTION ENGINE WORKS: ==================================== Translating ARM to x86 machine code: ------------------------------------ QEMU starts by isolating code "fragments" from the emulated machine code. Each "fragment" corresponds to a series of ARM instructions ending with a branch (e.g. jumps, conditional branches, returns). Each fragment is translated into a "translated block" (a.k.a. TB) of host machine code (e.g. x86). All TBs are put in a cache and each time the instruction pointer changes (i.e. at the end of TB execution), a hash table lookup is performed to find the next TB to execute. If none exists, a new one is generated. As a special exception, it is sometimes possible to 'link' the end of a given TB to the start of another one by tacking an explicit jump instruction. Note that due to differences in translations of memory-related operations (described below in "MMU emulation"), there are actually two TB caches per emulated CPU: one for translated kernel code, and one for translated user-space code. When a cache fills up, it is simply totally emptied and translation starts again. CPU state is kept in a single global structure which the generated code can access directly (with direct memory addressing). The file target-arm/translate.c is in charge of translating the ARM or Thumb instructions starting at the current instruction pointer position into a TB. This is done by decomposing each instruction into a series of micro-operations supported by the TCG code generator. TCG stands for "Tiny Code Generator" and is specific to QEMU. It supports several host machine code backends. See source files under tcg/ for details. MMU Emulation: -------------- The ARM Memory Management Unit is emulated in software, since it is so different from the one on the host. Essentially, a single ARM memory load/store instruction is translated into a series of host machine instructions that will translate virtual addresses into physical ones by performing the following: - first lookup in a global 256-entries cache for the current page and see if a corresponding value is already stored there. If this is the case, use it directly. - otherwise, call a special helper function that will implement the full translation according to the emulated system's state, and modify the cache accordingly. The page cache is called the "TLB" in the QEMU sources. Note that there are actually two TLBs: one is used for host machine instructions that correspond to kernel code, and the other for instructions translated from user-level code. This means that a memory load in the kernel will not be translated into the same instructions than the same load in user space. Each TLB is also implemented as a global per-emulated-CPU hash-table. The user-level TLB is flushed on each process context switch. When initializing the MMU emulation, one can define several zones of the address space, with different access rights / type. This is how memory-mapped I/O is implemented: the virtual->physical conversion helper function detects that you're trying to read/write from an I/O memory region, and will then call a callback function associated to it. Hardware Emulation: ------------------- Most hardware emulation code initializes by registering its own region of I/O memory, as well as providing read/write callbacks for it. Then actions will be based on which offset of the I/O memory is read from/written to and eventually with which value. You can have a look at hw/goldfish_tty.c that implements an emulated serial port for the Goldfish platform. "Goldfish" is simply the name of the virtual Linux platform used to build the Android-emulator-specific kernel image. The corresponding sources are located in the origin/android-goldfish-2.6.27 branch of git://android.git.kernel.org/kernel/common.git. You can have a look at arch/arm/mach-goldfish/ for the corresponding kernel driver sources. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 21:48 ` David Turner @ 2009-03-31 22:38 ` malc 2009-03-31 23:28 ` David Turner 0 siblings, 1 reply; 33+ messages in thread From: malc @ 2009-03-31 22:38 UTC (permalink / raw) To: qemu-devel On Tue, 31 Mar 2009, David Turner wrote: > On Tue, Mar 31, 2009 at 6:18 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > > > > > True enough for the comments part. There are still some areas that I > > don't understand well, for example TB handling and its inherent > > limitations. > > > > Which part of the source you find subtle and magical but not commented > > enough? > > > > ok, here are a few things that ring a bell, there are probably a lot others: > > the software mmu implementation in system mode is *really* hard to > understand at > first. It took me a long time to grasp the various aspects of it, including > these: > > - loads/stores in kernel or userspace map to different translated code > fragments > - loads/stores in different emulated CPUs also map to different > translated code. > - the way i/o memory access is controlled in fine details and relates to > the rest of the MMU. > > most of what happens in exe.c is black-magic :-) > > how the audio-subsystem works and its relationship with hardware > emulation is subtle. For my work on the Android emulator, I have > modified three audio-backends and wrote one from scratch. It took me > several tries to get things to an acceptable state. What I didn't > understand first is that there is no common time-based used by all > components involved. > For starters you could have asked about things you believe are subtle. Now to the part i do not understand: what do you mean by time-based(sic)? There's one clock involved, as far as audio is concerned, and it's the one dervied from the speed with which host audio can consume/produce audio, anything else just doesn't work (for scenarios i was interested in anyway) As for the question of why everything just calls audio_pcm_sw_write raised in AUDIO.txt, the reason, if my memory serves and it might as well not do that all that well, things are the way they are for the reason that any given driver can, theoretically, use the respective audio subsystems/libraries own, less naive, st_rate_flow equivalent. [..snip..] > For the record, here are attached a few documents I wrote to detail > what I've "discovered" until now. Hope some one can find them useful > (Sorry if some of them are focused on ARM system emulation only). > > Regards > P.S. Last/only time i looked at Android couldn't help but notice that, apart from other things, capture was implemented for coreaudio, it would have been nice, if for nothing else but the sake of completeness, to have that in main QEMU too. -- mailto:av1474@comtv.ru ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 22:38 ` malc @ 2009-03-31 23:28 ` David Turner 2009-03-31 23:49 ` malc 0 siblings, 1 reply; 33+ messages in thread From: David Turner @ 2009-03-31 23:28 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 3406 bytes --] On Wed, Apr 1, 2009 at 12:38 AM, malc <av1474@comtv.ru> wrote: > > For starters you could have asked about things you believe are subtle. > True enough, but for a long time the project was confidential and alluding to it publicly was not recommended. I'll try to break the habit. > > Now to the part i do not understand: what do you mean by time-based(sic)? > There's one clock involved, as far as audio is concerned, and it's the > one dervied from the speed with which host audio can consume/produce > audio, anything else just doesn't work (for scenarios i was interested > in anyway) > I had some problems when running qemu on low-powered computers. If I remember correctly, the emulation part was taking so much of the CPU that audio could very well stutter in strange ways on some platforms. One of the reason for it was that the host audio output (e.g. esd) stopped consuming samples at a consistent rate, which forced SWVoiceOut buffers to fill up and delay audio production in the emulated system even more. In certain cases, there is also a non-trivial latency between the moment the emulated system produces audio-samples (e.g. sends them through dma to the emulated hardware), and the moment it is effectively sent to the host backend. This is mostly related to playing with the audio timer tick configuration which by default is so small that it should not matter. What I mean by time-based means a way to deal with varying latencies in both audio production and consumption. I agree it's a hard problem and is not totally required for QEMU. It's just that I spent some time trying to understand how the audio subsystem dealt with the problem, except that it didn't. > > As for the question of why everything just calls audio_pcm_sw_write > raised in AUDIO.txt, the reason, if my memory serves and it might as > well not do that all that well, things are the way they are for the > reason that any given driver can, theoretically, use the respective > audio subsystems/libraries own, less naive, st_rate_flow equivalent. > Interesting, thanks for the explanation, this makes it clear. > P.S. Last/only time i looked at Android couldn't help but notice that, > apart from other things, capture was implemented for coreaudio, > it would have been nice, if for nothing else but the sake of > completeness, to have that in main QEMU too. > Actually, the details of audio-related changes performed are: - adding audio input to the CoreAudio backend - adding dynamic linking support to the esd and alsa backends (using dlopen/dlsym allows the emulator to run on platforms where all the corresponding libraries / sound server are not available). - rewriting the sdl backend run_out() method. For some reason the old one tended to lock up QEMU in certain weird cases I could never completely understand. - modifying the sub-system to be able to use different backends for audio input and output. - adding a new "winaudio.c" backend for Windows that uses Wave functions instead of DirectX (mainly to be able to build on old versions of mingw that didn't provide directx-compatible headers, I'm not sure if it's still needed these days, but at least the code is 10x simpler than the dxaudio.c one, talk about a complex sound API). I plan to provide patches for all of these for upstream. Sorry if this hasn't been done yet. > -- > mailto:av1474@comtv.ru > > > [-- Attachment #2: Type: text/html, Size: 4838 bytes --] ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 23:28 ` David Turner @ 2009-03-31 23:49 ` malc 2009-04-01 0:25 ` David Turner 0 siblings, 1 reply; 33+ messages in thread From: malc @ 2009-03-31 23:49 UTC (permalink / raw) To: qemu-devel On Wed, 1 Apr 2009, David Turner wrote: > On Wed, Apr 1, 2009 at 12:38 AM, malc <av1474@comtv.ru> wrote: > > > > > For starters you could have asked about things you believe are subtle. > > > > True enough, but for a long time the project was confidential and alluding > to it publicly > was not recommended. I'll try to break the habit. > > > > > > Now to the part i do not understand: what do you mean by time-based(sic)? > > > > There's one clock involved, as far as audio is concerned, and it's the > > one dervied from the speed with which host audio can consume/produce > > audio, anything else just doesn't work (for scenarios i was interested > > in anyway) > > > > I had some problems when running qemu on low-powered computers. If I > remember correctly, the emulation part was taking so much of the CPU > that audio could very well stutter in strange ways on some > platforms. One of the reason for it was that the host audio output > (e.g. esd) stopped consuming samples at a consistent rate, which > forced SWVoiceOut buffers to fill up and delay audio production in > the emulated system even more. Hmm.. please define low-powered.. Audio started being developed on a 1Ghz Athlon, granted the machine was equipped with Aureal Vortex 2 and i had a luxory of using [ADA]C_FIXED_SETTINGS set to zero and DAC_VOICES set to something reasonable thus basically avoiding software mixing altogether, that said some of the old DOS games/demos use Soundblaster/OPL[23] in such creative ways so doing things in software even with au8830 was sometimes inevitable... And FWIW nowadays it's MPC7447A at 1.3Ghz which is not speed demon either. Not that i dont belive you, esp considering esd thrown into equation, but it would be interesting to know what people deem low-powered currently. > In certain cases, there is also a non-trivial latency between the > moment the emulated system produces audio-samples (e.g. sends them > through dma to the emulated hardware), and the moment it is > effectively sent to the host backend. This is mostly related to > playing with the audio timer tick configuration which by default is > so small that it should not matter. > > What I mean by time-based means a way to deal with varying latencies > in both audio production and consumption. I agree it's a hard > problem and is not totally required for QEMU. It's just that I spent > some time trying to understand how the audio subsystem dealt with > the problem, except that it didn't. Well.. I certainly fail to see how adding some other clock would overcome the fact that something can't keep up.. (rt priorties and suchlike?) > > > > > As for the question of why everything just calls audio_pcm_sw_write > > raised in AUDIO.txt, the reason, if my memory serves and it might as > > well not do that all that well, things are the way they are for the > > reason that any given driver can, theoretically, use the respective > > audio subsystems/libraries own, less naive, st_rate_flow equivalent. > > > > Interesting, thanks for the explanation, this makes it clear. > > > > P.S. Last/only time i looked at Android couldn't help but notice that, > > apart from other things, capture was implemented for coreaudio, > > it would have been nice, if for nothing else but the sake of > > completeness, to have that in main QEMU too. > > > > Actually, the details of audio-related changes performed are: > > - adding audio input to the CoreAudio backend > > - adding dynamic linking support to the esd and alsa backends > (using dlopen/dlsym allows the emulator to run on platforms where > all the corresponding libraries / sound server are not available). I don't think this is worth it for QEMU, after all shipping binaries is not what it's best known for. > - rewriting the sdl backend run_out() method. For some reason the old one > tended to lock up QEMU in certain weird cases I could never > completely understand. Hmm.. > - modifying the sub-system to be able to use different backends for > audio input and output. Yeah i recall seeing this, and was wondering why Android needed this functionality. > - adding a new "winaudio.c" backend for Windows that uses Wave > functions instead of DirectX (mainly to be able to build on old > versions of mingw that didn't provide directx-compatible headers, > I'm not sure if it's still needed these days, but at least the > code is 10x simpler than the dxaudio.c one, talk about a complex > sound API). > Complexity notwithstanding i happen to like the way things are done in DSound, the conceptually simple approach that is. > I plan to provide patches for all of these for upstream. Sorry if this > hasn't been done yet. > Nice to hear that. -- mailto:av1474@comtv.ru ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 23:49 ` malc @ 2009-04-01 0:25 ` David Turner 2009-04-01 1:02 ` malc 0 siblings, 1 reply; 33+ messages in thread From: David Turner @ 2009-04-01 0:25 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 3207 bytes --] On Wed, Apr 1, 2009 at 1:49 AM, malc <av1474@comtv.ru> wrote: > Hmm.. please define low-powered.. No problem, people have been running the Android SDK on 700 MHz. I used to run it on a 900Hz Celeron or something like that. Today, I'm doing all my "minimal performance" tests on a venerable 1 GHz Pentium III laptop with an integrated Intel audio chipset running Windows XP (and a Linux install under VMWare). Just to be clear, I'm not suggesting to anyone to fix anything here.. > > And FWIW nowadays it's MPC7447A at 1.3Ghz which is not speed demon > either. Not that i dont belive you, esp considering esd thrown into > equation, but it would be interesting to know what people deem > low-powered currently. > Regarding esd, I unfortunately have to support the emulator binary running on Ubuntu 6.06 installations where the sound server will frequently lock up when the Android emulator is run. In some cases, this also totally freezes the whole desktop, though it is possible to recover by perfoming a "killall -9 esd" from a console. I initially thought that the main reason for that was that the esd client library could not handle the multiple SIGALRM-induced EINTR returned by write() and other system calls, and left the sound server in a sorry state (maybe because it was stuck waiting for some data that would never arrive). However, I protected all calls to the esd backend (playing with the signal mask to avoid that), but this still doesn't get rid of the problem. Fortunately, this doesn't seem to happen with later versions of Ubuntu. If anyone has an explanation for this behaviour (which doesn't seem to happen on later Ubuntu releases), I'd be happy to share more info. Well.. I certainly fail to see how adding some other clock would > overcome the fact that something can't keep up.. (rt priorties and > suchlike?) > the main idea is to avoid filling up buffers by dropping some frames when that kind of thing happens. Or to introduce silence in the output under other conditions. The main idea is to avoid increasing drift between the emulated and host system when it comes to audio. Again, I'm not suggesting to implement anything like that. > > > > - adding dynamic linking support to the esd and alsa backends > > (using dlopen/dlsym allows the emulator to run on platforms where > > all the corresponding libraries / sound server are not available). > > I don't think this is worth it for QEMU, after all shipping binaries > is not what it's best known for. > :-) > > > - modifying the sub-system to be able to use different backends for > > audio input and output. > > Yeah i recall seeing this, and was wondering why Android needed this > functionality. > this was to be able to get audio input from a wave file while sending the output to the host audio system. Turned out to be very useful to test the VoiceRecorder application. I also played with it to debug esd/also related problems. It's not exactly something that is worth it for upstream QEMU. > Complexity notwithstanding i happen to like the way things are done in > DSound, the conceptually simple approach that is. > I must admit I totally fail to see any simplicity in DirectSound :-) Regards [-- Attachment #2: Type: text/html, Size: 4697 bytes --] ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-04-01 0:25 ` David Turner @ 2009-04-01 1:02 ` malc 0 siblings, 0 replies; 33+ messages in thread From: malc @ 2009-04-01 1:02 UTC (permalink / raw) To: qemu-devel On Wed, 1 Apr 2009, David Turner wrote: > On Wed, Apr 1, 2009 at 1:49 AM, malc <av1474@comtv.ru> wrote: > > > Hmm.. please define low-powered.. > > > No problem, people have been running the Android SDK on 700 MHz. > I used to run it on a 900Hz Celeron or something like that. > > Today, I'm doing all my "minimal performance" tests on a venerable > 1 GHz Pentium III laptop with an integrated Intel audio chipset running > Windows XP (and a Linux install under VMWare). > > Just to be clear, I'm not suggesting to anyone to fix anything here.. > > > > > > And FWIW nowadays it's MPC7447A at 1.3Ghz which is not speed demon > > either. Not that i dont belive you, esp considering esd thrown into > > equation, but it would be interesting to know what people deem > > low-powered currently. > > This thing is actually slower than the old Athlon.. Perhaps on par/slightly faster than 1Ghz Pentium III.. And if ARM Android emulates is big-endian - Pentium has some advantage even.. > Regarding esd, I unfortunately have to support the emulator binary > running on Ubuntu 6.06 installations where the sound server will > frequently lock up when the Android emulator is run. In some cases, > this also totally freezes the whole desktop, though it is possible > to recover by perfoming a "killall -9 esd" from a console. I have very hard time parsing that, you are suspecting emulator binary locking up esd daemon? > I initially thought that the main reason for that was that the esd > client library could not handle the multiple SIGALRM-induced EINTR > returned by write() and other system calls, and left the sound > server in a sorry state (maybe because it was stuck waiting for some > data that would never arrive). However, I protected all calls to the > esd backend (playing with the signal mask to avoid that), but this > still doesn't get rid of the problem. Fortunately, this doesn't seem > to happen with later versions of Ubuntu. esdaudio.c blocks all signals for both playback and recording.. So hum.. > If anyone has an explanation for this behaviour (which doesn't seem > to happen on later Ubuntu releases), I'd be happy to share more > info. > > > Well.. I certainly fail to see how adding some other clock would > > overcome the fact that something can't keep up.. (rt priorties and > > suchlike?) > > > > the main idea is to avoid filling up buffers by dropping some frames > when that kind of thing happens. Or to introduce silence in the > output under other conditions. The main idea is to avoid increasing > drift between the emulated and host system when it comes to > audio. Again, I'm not suggesting to implement anything like that. > Drift can only grow as big as the ring buffer in any case... > > > > > > - adding dynamic linking support to the esd and alsa backends > > > (using dlopen/dlsym allows the emulator to run on platforms where > > > all the corresponding libraries / sound server are not available). > > > > I don't think this is worth it for QEMU, after all shipping binaries > > is not what it's best known for. > > > > :-) > > > > > > > - modifying the sub-system to be able to use different backends for > > > audio input and output. > > > > Yeah i recall seeing this, and was wondering why Android needed this > > functionality. > > > > this was to be able to get audio input from a wave file while > sending the output to the host audio system. Turned out to be very > useful to test the VoiceRecorder application. I also played with it > to debug esd/also related problems. It's not exactly something that > is worth it for upstream QEMU. Oh.. I never needed that, otherwise there would probably be a wavplayback alongside wavcapture monitor command. > > > Complexity notwithstanding i happen to like the way things are done in > > DSound, the conceptually simple approach that is. > > > > I must admit I totally fail to see any simplicity in DirectSound :-) Oh it's just overly complicated oss+mmap with locking and the ability to get the current position, FMOD is almost exactly the same but the wrapping is more palatable. P.S. Your mails (raw text versions of them) have their lines wrapped weirdly. -- mailto:av1474@comtv.ru ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-31 12:58 ` David Turner 2009-03-31 13:31 ` Avi Kivity 2009-03-31 16:18 ` Blue Swirl @ 2009-04-01 9:04 ` Daniel P. Berrange 2 siblings, 0 replies; 33+ messages in thread From: Daniel P. Berrange @ 2009-04-01 9:04 UTC (permalink / raw) To: qemu-devel On Tue, Mar 31, 2009 at 02:58:50PM +0200, David Turner wrote: > Very frankly, I don't think that a coding style, even strictly applied, is > going to make the QEMU code > easier to understand. > > The real barriers to understanding are the lack of structure in the code, > liberal use of global macros > scattered randomly in the source code, exceedingly liberally named > functions, and sometimes obscure > implementation of simple concepts (*cough* CharDriverState), cramming > totally unrelated stuff in single > largish source files (vl.c for the win !), and a blatant lack of > documentation comments for a lot of subtle > stuff in there to explain the magic. Much of that is true, but there has been very active work addressing these problems in recent times. If you look at the history of vl.c for example, you'll see it has dropped from 10,000 lines to just 5,800 today, with much code split out to separate modules. There's of course much more still todo in this area, but this is no reason to not try and keep a clean & consistent coding style at the same time as this refactoring. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 18:28 ` Blue Swirl 2009-03-30 19:02 ` M. Warner Losh 2009-03-30 19:54 ` Avi Kivity @ 2009-03-30 19:58 ` Avi Kivity 2009-03-30 20:10 ` Glauber Costa 2009-03-30 20:20 ` Andreas Färber 2009-03-30 21:45 ` Lennart Sorensen 4 siblings, 1 reply; 33+ messages in thread From: Avi Kivity @ 2009-03-30 19:58 UTC (permalink / raw) To: qemu-devel Blue Swirl wrote: >> +Rationale: a consistent (except for functions...) bracing style reduces >> +ambiguity and avoids needless churn when lines are added or removed. >> +Furthermore, it is the Qemu coding style. >> > > No, this is the K&R style. Quoting linux/Documentation/CodingStyle: > No, it is the Qemu^WQEMU coding style. QEMU. QEMU. QEMUUUUUUUUUUUUUUUUUU! Mu. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 19:58 ` Avi Kivity @ 2009-03-30 20:10 ` Glauber Costa 2009-03-30 20:35 ` Avi Kivity 0 siblings, 1 reply; 33+ messages in thread From: Glauber Costa @ 2009-03-30 20:10 UTC (permalink / raw) To: qemu-devel On Mon, Mar 30, 2009 at 4:58 PM, Avi Kivity <avi@redhat.com> wrote: > Blue Swirl wrote: >>> >>> +Rationale: a consistent (except for functions...) bracing style reduces >>> +ambiguity and avoids needless churn when lines are added or removed. >>> +Furthermore, it is the Qemu coding style. >>> >> >> No, this is the K&R style. Quoting linux/Documentation/CodingStyle: >> > > No, it is the Qemu^WQEMU coding style. QEMU. QEMU. QEMUUUUUUUUUUUUUUUUUU! > > Mu. Avi, have you been drinking? -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act." ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 20:10 ` Glauber Costa @ 2009-03-30 20:35 ` Avi Kivity 2009-03-30 20:37 ` Glauber Costa 0 siblings, 1 reply; 33+ messages in thread From: Avi Kivity @ 2009-03-30 20:35 UTC (permalink / raw) To: qemu-devel Glauber Costa wrote: > have you been drinking? > No. Why do you ask? -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 20:35 ` Avi Kivity @ 2009-03-30 20:37 ` Glauber Costa 0 siblings, 0 replies; 33+ messages in thread From: Glauber Costa @ 2009-03-30 20:37 UTC (permalink / raw) To: qemu-devel On Mon, Mar 30, 2009 at 5:35 PM, Avi Kivity <avi@redhat.com> wrote: > Glauber Costa wrote: >> >> have you been drinking? >> > > No. Why do you ask? I would like to have some. -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act." ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 18:28 ` Blue Swirl ` (2 preceding siblings ...) 2009-03-30 19:58 ` Avi Kivity @ 2009-03-30 20:20 ` Andreas Färber 2009-03-30 21:45 ` Lennart Sorensen 4 siblings, 0 replies; 33+ messages in thread From: Andreas Färber @ 2009-03-30 20:20 UTC (permalink / raw) To: qemu-devel Am 30.03.2009 um 20:28 schrieb Blue Swirl: > Besides, functions are > special anyway (you can't nest them in C). Nesting functions used to work in GCC... :) ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 18:28 ` Blue Swirl ` (3 preceding siblings ...) 2009-03-30 20:20 ` Andreas Färber @ 2009-03-30 21:45 ` Lennart Sorensen 2009-03-30 22:16 ` M. Warner Losh 2009-03-31 5:42 ` Gleb Natapov 4 siblings, 2 replies; 33+ messages in thread From: Lennart Sorensen @ 2009-03-30 21:45 UTC (permalink / raw) To: qemu-devel On Mon, Mar 30, 2009 at 09:28:54PM +0300, Blue Swirl wrote: > I'd remove this, braces are not used consistently for one statement blocks. Then fix that. Making the coding style worse just because some people have failed to follow it is not the right solution. > No, this is the K&R style. Quoting linux/Documentation/CodingStyle: > > Heretic people all over the world have claimed that this inconsistency > is ... well ... inconsistent, but all right-thinking people know that > (a) K&R are _right_ and (b) K&R are right. Besides, functions are > special anyway (you can't nest them in C). And Linus is wrong for not requiring braces at all times in the kernel. :) I don't have much hope of convincing him of that, but he is still wrong. -- Len Sorensen ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 21:45 ` Lennart Sorensen @ 2009-03-30 22:16 ` M. Warner Losh 2009-03-31 5:42 ` Gleb Natapov 1 sibling, 0 replies; 33+ messages in thread From: M. Warner Losh @ 2009-03-30 22:16 UTC (permalink / raw) To: qemu-devel, lsorense In message: <20090330214547.GQ3795@csclub.uwaterloo.ca> lsorense@csclub.uwaterloo.ca (Lennart Sorensen) writes: : On Mon, Mar 30, 2009 at 09:28:54PM +0300, Blue Swirl wrote: : > I'd remove this, braces are not used consistently for one statement blocks. : : Then fix that. Making the coding style worse just because some people : have failed to follow it is not the right solution. : : > No, this is the K&R style. Quoting linux/Documentation/CodingStyle: : > : > Heretic people all over the world have claimed that this inconsistency : > is ... well ... inconsistent, but all right-thinking people know that : > (a) K&R are _right_ and (b) K&R are right. Besides, functions are : > special anyway (you can't nest them in C). : : And Linus is wrong for not requiring braces at all times in the kernel. :) : I don't have much hope of convincing him of that, but he is still wrong. And this is an example of the religious nature of this argument :) Warner ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-30 21:45 ` Lennart Sorensen 2009-03-30 22:16 ` M. Warner Losh @ 2009-03-31 5:42 ` Gleb Natapov 1 sibling, 0 replies; 33+ messages in thread From: Gleb Natapov @ 2009-03-31 5:42 UTC (permalink / raw) To: qemu-devel On Mon, Mar 30, 2009 at 05:45:47PM -0400, Lennart Sorensen wrote: > On Mon, Mar 30, 2009 at 09:28:54PM +0300, Blue Swirl wrote: > > I'd remove this, braces are not used consistently for one statement blocks. > > Then fix that. Making the coding style worse just because some people > have failed to follow it is not the right solution. > > > No, this is the K&R style. Quoting linux/Documentation/CodingStyle: > > > > Heretic people all over the world have claimed that this inconsistency > > is ... well ... inconsistent, but all right-thinking people know that > > (a) K&R are _right_ and (b) K&R are right. Besides, functions are > > special anyway (you can't nest them in C). > > And Linus is wrong for not requiring braces at all times in the kernel. :) > I don't have much hope of convincing him of that, but he is still wrong. > Try it anyway. Do it on LKML. It will be entertaining. -- Gleb. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-29 21:23 [Qemu-devel] [PATCH] Document Qemu coding style Avi Kivity 2009-03-30 1:15 ` malc 2009-03-30 18:28 ` Blue Swirl @ 2009-03-31 13:47 ` Paul Brook 2009-04-01 8:51 ` Richard W.M. Jones 3 siblings, 0 replies; 33+ messages in thread From: Paul Brook @ 2009-03-31 13:47 UTC (permalink / raw) To: qemu-devel; +Cc: Avi Kivity > +2. Line width > + > +Lines are 80 characters wide plus some slop. Try to fit your code into > +eighty characters, but if it makes a snippet particularly ugly, allow > +yourself some slack. Don't overdo it though. I'd remove the "plus some slop". My guess is that most people using a non-graphical editor (and probably many of those using graphical variants of vi/emacs) still wrap at 80 characters. If you can't find a convenient place to break the line, then maybe you're doing something else wrong (e.g. using 40-character variable names :-) Other than that, looks ok. Paul ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-03-29 21:23 [Qemu-devel] [PATCH] Document Qemu coding style Avi Kivity ` (2 preceding siblings ...) 2009-03-31 13:47 ` Paul Brook @ 2009-04-01 8:51 ` Richard W.M. Jones 2009-04-01 9:04 ` Avi Kivity 3 siblings, 1 reply; 33+ messages in thread From: Richard W.M. Jones @ 2009-04-01 8:51 UTC (permalink / raw) To: qemu-devel; +Cc: avi On Mon, Mar 30, 2009 at 12:23:43AM +0300, Avi Kivity wrote: > With the help of some Limoncino I noted several aspects of the Qemu coding > style, particularly where it differs from the Linux coding style as many > contributors work on both projects. > > Signed-off-by: Avi Kivity <avi@redhat.com> > --- > CODING_STYLE | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 77 insertions(+), 0 deletions(-) > create mode 100644 CODING_STYLE > > diff --git a/CODING_STYLE b/CODING_STYLE Just as a general comment, have you thought about providing specific guidance for emacs and vi users? In libvirt, the HACKING file contains the following useful snippet for emacs users: http://git.et.redhat.com/?p=libvirt.git;a=blob;f=HACKING;h=ca39d61d85f2aaa3991f13a540772932208be7cd;hb=f5c687f03d8313407448fc642f8fefe95a655645#l59 They just paste that into their ~/.emacs, and any time they edit libvirt code it "just works". Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH] Document Qemu coding style 2009-04-01 8:51 ` Richard W.M. Jones @ 2009-04-01 9:04 ` Avi Kivity 0 siblings, 0 replies; 33+ messages in thread From: Avi Kivity @ 2009-04-01 9:04 UTC (permalink / raw) To: Richard W.M. Jones; +Cc: qemu-devel Richard W.M. Jones wrote: > On Mon, Mar 30, 2009 at 12:23:43AM +0300, Avi Kivity wrote: > >> With the help of some Limoncino I noted several aspects of the Qemu coding >> style, particularly where it differs from the Linux coding style as many >> contributors work on both projects. >> > Just as a general comment, have you thought about providing specific > guidance for emacs and vi users? > > In libvirt, the HACKING file contains the following useful snippet for > emacs users: > > http://git.et.redhat.com/?p=libvirt.git;a=blob;f=HACKING;h=ca39d61d85f2aaa3991f13a540772932208be7cd;hb=f5c687f03d8313407448fc642f8fefe95a655645#l59 > > They just paste that into their ~/.emacs, and any time they edit > libvirt code it "just works". > > Good idea. Maybe not in CODING_STYLE, but it's certainly helpful. I use something similar in my ~/.emacs. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2009-04-01 9:22 UTC | newest] Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-03-29 21:23 [Qemu-devel] [PATCH] Document Qemu coding style Avi Kivity 2009-03-30 1:15 ` malc 2009-03-30 18:28 ` Blue Swirl 2009-03-30 19:02 ` M. Warner Losh 2009-03-30 19:55 ` Avi Kivity 2009-03-30 19:54 ` Avi Kivity 2009-03-30 21:43 ` Lennart Sorensen 2009-03-30 22:15 ` M. Warner Losh 2009-03-30 23:38 ` Lennart Sorensen 2009-03-31 0:09 ` M. Warner Losh 2009-03-31 5:59 ` Laurent Desnogues 2009-03-31 12:58 ` David Turner 2009-03-31 13:31 ` Avi Kivity 2009-03-31 21:18 ` David Turner 2009-03-31 16:18 ` Blue Swirl 2009-03-31 21:48 ` David Turner 2009-03-31 22:38 ` malc 2009-03-31 23:28 ` David Turner 2009-03-31 23:49 ` malc 2009-04-01 0:25 ` David Turner 2009-04-01 1:02 ` malc 2009-04-01 9:04 ` Daniel P. Berrange 2009-03-30 19:58 ` Avi Kivity 2009-03-30 20:10 ` Glauber Costa 2009-03-30 20:35 ` Avi Kivity 2009-03-30 20:37 ` Glauber Costa 2009-03-30 20:20 ` Andreas Färber 2009-03-30 21:45 ` Lennart Sorensen 2009-03-30 22:16 ` M. Warner Losh 2009-03-31 5:42 ` Gleb Natapov 2009-03-31 13:47 ` Paul Brook 2009-04-01 8:51 ` Richard W.M. Jones 2009-04-01 9:04 ` Avi Kivity
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.