| Message ID | AANLkTikqSLRUw9WYVQoA9tbaaa9tgNjYmrRV-EEv7DbW@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 12 Aug 2010, Blue Swirl wrote: > Add preprocessor rules from libvirt HACKING. > > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > --- > CODING_STYLE | 13 +++++++++++++ > 1 files changed, 13 insertions(+), 0 deletions(-) > > diff --git a/CODING_STYLE b/CODING_STYLE > index 92036f3..c4c09ab 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -79,3 +79,16 @@ and clarity it comes on a line by itself: > 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. > + > +5. Preprocessor > + > +For variadic macros, stick with C99 syntax: > + > +#define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__) _ctl is not used inside the macro expansion, furthermore i'd avoid using leading underscore even for macro arguments. > + > +Use parenthesis when checking if a macro is defined, and use > +indentation to track nesting: > + > +#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE) > +# define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) > +#endif >
On Thu, Aug 12, 2010 at 6:47 PM, malc <av1474@comtv.ru> wrote: > On Thu, 12 Aug 2010, Blue Swirl wrote: > >> Add preprocessor rules from libvirt HACKING. >> >> Signed-off-by: Blue Swirl <blauwirbel@gmail.com> >> --- >> CODING_STYLE | 13 +++++++++++++ >> 1 files changed, 13 insertions(+), 0 deletions(-) >> >> diff --git a/CODING_STYLE b/CODING_STYLE >> index 92036f3..c4c09ab 100644 >> --- a/CODING_STYLE >> +++ b/CODING_STYLE >> @@ -79,3 +79,16 @@ and clarity it comes on a line by itself: >> 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. >> + >> +5. Preprocessor >> + >> +For variadic macros, stick with C99 syntax: >> + >> +#define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__) > > _ctl is not used inside the macro expansion, furthermore i'd avoid > using leading underscore even for macro arguments. Right. Moreover the macro name is too libvirt'ish. >> + >> +Use parenthesis when checking if a macro is defined, and use >> +indentation to track nesting: >> + >> +#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE) >> +# define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) >> +#endif This one is new, current code doesn't use indentation. Maybe it's better to drop it.
Am 13.08.2010 um 18:56 schrieb Blue Swirl: >>> +Use parenthesis when checking if a macro is defined, and use >>> +indentation to track nesting: >>> + >>> +#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE) >>> +# define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) >>> +#endif > > This one is new, current code doesn't use indentation. Maybe it's > better to drop it. Yes, please. While indentation is nice to read, it is patch-unfriendly. Consider: #ifdef SOMETHINGNEW yay #else // previous stuff follows #if defined(OLDONE) # define one #endif foo bar #if defined(OLDTWO) # define two #endif #endif Here, adding four lines would require to reindent six lines otherwise not changed, making merging and review harder. Andreas
On Sat, Aug 14, 2010 at 2:27 PM, Andreas Färber <andreas.faerber@web.de> wrote: > Am 13.08.2010 um 18:56 schrieb Blue Swirl: > >>>> +Use parenthesis when checking if a macro is defined, and use >>>> +indentation to track nesting: >>>> + >>>> +#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE) >>>> +# define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) >>>> +#endif >> >> This one is new, current code doesn't use indentation. Maybe it's >> better to drop it. > > Yes, please. While indentation is nice to read, it is patch-unfriendly. > > Consider: > > #ifdef SOMETHINGNEW > yay > #else // previous stuff follows > > #if defined(OLDONE) > # define one > #endif > foo > bar > #if defined(OLDTWO) > # define two > #endif > > #endif > > Here, adding four lines would require to reindent six lines otherwise not > changed, making merging and review harder. I don't quite follow the example, but is the situation different from adding a C 'if' statement and adjusting the indentation? However, I'm not proposing this anymore.
Am 15.08.2010 um 09:46 schrieb Blue Swirl: > On Sat, Aug 14, 2010 at 2:27 PM, Andreas Färber <andreas.faerber@web.de > > wrote: >> While indentation is nice to read, it is patch-unfriendly. >> >> Consider: >> >> #ifdef SOMETHINGNEW >> yay >> #else // previous stuff follows >> >> #if defined(OLDONE) >> # define one >> #endif >> foo >> bar >> #if defined(OLDTWO) >> # define two >> #endif >> >> #endif >> >> Here, adding four lines would require to reindent six lines >> otherwise not >> changed, making merging and review harder. > > I don't quite follow the example, The middle part would need to become: # if defined(OLDONE) # define one # endif foo bar # if defined(OLDTWO) # define two # endif if we applied the rule. Purely syntactic example. > but is the situation different from > adding a C 'if' statement and adjusting the indentation? No, you're right, it isn't. Just that we previously applied such changes for Windows or Solaris and did not need to reindent. Cheers, Andreas
diff --git a/CODING_STYLE b/CODING_STYLE index 92036f3..c4c09ab 100644 --- a/CODING_STYLE +++ b/CODING_STYLE @@ -79,3 +79,16 @@ and clarity it comes on a line by itself: 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. + +5. Preprocessor + +For variadic macros, stick with C99 syntax: + +#define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__) + +Use parenthesis when checking if a macro is defined, and use +indentation to track nesting: + +#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE) +# define fallocate(a,ignored,b,c) posix_fallocate(a,b,c) +#endif
Add preprocessor rules from libvirt HACKING. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- CODING_STYLE | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)