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

Andrew Morton
2006-06-08T01:28:22Z

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'