Messages in this thread |  | | Date | Wed, 7 Jun 2006 18:28:08 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] fix generic HDLC synclink mismatch build error |
| |
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' - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |