linux-kernel - Re: linux-next20080314 build fails with !CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 14 Mar 2008 15:05:54 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Kamalesh Babulal <kamalesh@...ux.vnet.ibm.com>
Cc:	linux-next@...r.kernel.org, sfr@...b.auug.org.au,
	linux-kernel@...r.kernel.org, apw@...dowen.org,
	Alan Stern <stern@...land.harvard.edu>,
	Len Brown <lenb@...nel.org>
Subject: Re: linux-next20080314 build fails with !CONFIG_PM

On Fri, 14 Mar 2008 13:27:10 +0530
Kamalesh Babulal <kamalesh@...ux.vnet.ibm.com> wrote:

> The next-20080314 tree build fails 
> 
> drivers/serial/serial_core.c: In function `uart_add_one_port':
> drivers/serial/serial_core.c:2359: error: invalid lvalue in assignment
> make[2]: *** [drivers/serial/serial_core.o] Error 1
> 
> The config # CONFIG_PM was not set.The code which is causing the 
> build failure is 
> 
> 	device_can_wakeup(tty_dev) = 1;
> 
> when the CONFIG_PM is set the macro is preprocessed as
> 
> #define device_can_wakeup(dev) \
>         ((dev)->power.can_wakeup)
> 
> and when not set, it becomes 0 = 1 
> 
> #define device_can_wakeup(dev)                  0
> 

Caused by Alan's "PM: make wakeup flags available whenever CONFIG_PM is
set" which I assume Len merged.

Sorry, but that code should be dragged out and shot.  Doing this:

> 	device_can_wakeup(tty_dev) = 1;

is just gross and stupid.  It looks daft, it isn't C and it *requires* that
device_can_wakeup() be implemented as a macro, which totally busts any
concept of abstraction.

The code previously had quite reasonable accessors:

 #define device_set_wakeup_enable(dev,val) \
        ((dev)->power.should_wakeup = !!(val))
 #define device_may_wakeup(dev) \
        (device_can_wakeup(dev) && (dev)->power.should_wakeup)

can we please go back to that scheme?  Please also convert all these
magical macros into inlined C functions.  One reason is that this:

+#define device_init_wakeup(dev,val) \
+       do { \
+               device_can_wakeup(dev) = !!(val); \
+               device_set_wakeup_enable(dev,val); \
+       } while(0)

is buggy.  It evaluates its arguments multiple times and will cause
failures when passed expressions which have side-effects.

Alan, please also pass all future patches through checkpatch - there is no
need to be adding trivial coding-style errors in this day and age.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/