Re: [MiNT] Patch: shel_write() did not call the PureC patcher

[Eero Tamminen <oak@helsinkinet.fi>]

Hi,

On keskiviikko 21 maaliskuu 2012, Jo Even Skarstein wrote:
> On Wed, 2012-03-21 at 20:24 +0200, Eero Tamminen wrote:
> > I would also suggest using something similar as what Linux kernel does
> > to make code cleaner:
> > ----header-----
> > #ifdef COLDFIRE
> > extern void   patch_memset_purec(BASEPAGE *b);
> > #else
> > #define patch_memset_purec(b)
> > #endif
> > ----c-files----
> >     patch_memset_purec(b);
> > ---------------
> > 
> > This way the ifdef stuff is localized to appropriate headers and
> > C-files are clean.
> 
> In what way does this make the source cleaner? From what I can see the
> only thing you accomplish with this is to hide the fact that this
> function is only used on a Coldfire CPU.

Yes, the function name is also bad, it should reflect the fact
that it's ColdFire only.   It could be renamed to something like
"coldfire_patch_memset_purec" (or something shorter, but still
understandable).

The header name should also reflect the fact that it contains
ColdFire only functionality.


If you have only few ifdefs and they don't nest, this doesn't
matter so much, but with large number of conditional build
options, they start to have serious impact on readability.

Instead of define, one could also use empty static function,
like mentioned here:
	http://www.linuxjournal.com/article/5780?page=0,3


(As this isn't part of official Linux kernel coding guidelines[1],
I think it's more like a recommendation from some of the core
kernel maintaintainers.)


	- Eero

[1] http://www.kernel.org/doc/Documentation/CodingStyle