Re: [PATCH] fix generic HDLC synclink mismatch build error

[Andrew Morton <akpm@osdl.org>]

On Wed, 07 Jun 2006 19:59:05 -0500
Paul Fulghum <paulkf@microgate.com> wrote:

> Andrew:
> 
> Maybe you can lend some insight on what I should do.
> 
> I have posted multiple, working patches to correct
> the build errors resultsing from random kernel configs.
> 
> But we appear to be in perpetual micromanagement by committee
> mode where different people are giving conflicting
> feedback of the "that's ugly" or "you shouldn't do that" kind.
> 
> I'm happy to accept any *patch* anyone wants to post
> that corrects the build errors *and* does not break
> the driver by removing the ability to optionally include
> generic HDLC support in the synclink drivers *and*
> is accepted by everyone here. Nothing that meets
> those requirements has been posted yet.
> 
> (Randy's last patch comes as close as my last patch, but
> Jeff says any code using conditional configuration is wrong
> so that removes any patch posted so far)
> 
> I'm also happy to accept the status quo, the
> driver works fine.
> 
> So where do we go from here?

Well your patch looked reasonable, except it muddies the CONFIG_foo
namespace.  So could you rework it thusly:


- Change all instances of

#ifdef CONFIG_HDLC
	<stuff>
#endif

to

#ifdef HDLC_ENABLED
	<stuff>
#endif


then do

#if defined(CONFIG_HDLC) || (defined(CONFIG_HDLC_MODULE) && defined(CONFIG_SYNCLINK_MODULE))
#define HDLC_ENABLED
#endif

or whatever?



Or, better, (but semi off-topic):


#if HDLC_ENABLED
	<stuff>
#endif

#if defined(CONFIG_HDLC) || (defined(CONFIG_HDLC_MODULE) && defined(CONFIG_SYNCLINK_MODULE))
#define HDLC_ENABLED 1
#else
#define HDLC_ENABLED 0
#endif

Because then a) you'll get a warning if someone already defined
HDLC_ENABLED and b) you'll get a warning if you mistype `#if HLDC_ENABLED'