| Submitter | Blue Swirl |
|---|---|
| Date | Aug. 12, 2010, 5:49 p.m. |
| Message ID | <AANLkTikqSLRUw9WYVQoA9tbaaa9tgNjYmrRV-EEv7DbW@mail.gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/61637/ |
| State | New |
| Headers | show |
Comments
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
Patch
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(-)