[coreboot] [patch] Cleanups in Makefile for flashrom and superiotool

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Sun May 18 15:46:18 CEST 2008


On 18.05.2008 15:21, Peter Lemenkov wrote:
> 2008/5/18 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>:
>
>   
>>>> I can understand setting PREFIX, but the reason why you need to redefine
>>>> CFLAGS is a mystery to me.
>>>>         
>
>   
>>> We use our custom CFLAGS.
>>>       
>
>   
>>> -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
>>> -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386
>>> -mtune=generic -fasynchronous-unwind-tables
>>>       
>
>   
>> And these custom CFLAGS are missing -DDISABLE_DOC in the flashrom case
>> (harmful). Furthermore, they are missing $(SVNDEF) for superiotool
>> (wrong, but only a nuisance).
>>     
>
> No.
>
> CFLAGS='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
> -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386
> -mtune=generic -fasynchronous-unwind-tables
> -D'\''FLASHROM_VERSION="3332"'\'' -DDISABLE_DOC'
>
> We provide them in form of CFLAGC=%some_predefined_part%
> -D'\''FLASHROM_VERSION="3332"'\'' -DDISABLE_DOC'
>
> So we do define FLASHROM_VERSION and DISABLE_DOC.
>   

OK, you may do that, but what happens if someone defines CFLAGS='-O2'
and leaves it at that? With the patch you proposed, -DDISABLE_DOC would
be missing then.


> But, as I mentioned above, the best way to do so is to define then in
> a conditional way.
>   

-DDISABLE_DOC must be specified regardless of the CFLAGS passed in. I'm
sure there are ways to do that. Having $SVNDEF in the unconditional
CFLAGS is desirable as well. You could make SVNDEF a conditional define
to solve problems with that.


>>>> Will the modified install section work with a non-FSF install program as
>>>> well, i.e. is it truly cross-platform?
>>>>         
>
>   
>>> No idea.
>>>       
>
>   
>> Sorry, then that part is a no-go. We are currently preparing flashrom
>> for FreeBSD compatibility and I don't want to ruin that.
>>     
>
> OK, understood and agree with this.
>
> Apart from install-section other things (trick with conditional defines) are OK?
>   

If you can solve the conditional define problem as outlined above, I
think the patch can be Acked. Makefile stuff is very brittle and we had
several accidents in the past, so getting 2 Acks is a minimum.

Regards,
Carl-Daniel




More information about the coreboot mailing list