Re: eCos 3.0 beta 1 remaining issues

[John Dallaway <john@dallaway.org.uk>]

Hi Bart

Bart Veer wrote:

>>>>>> "John" == John Dallaway <john@dallaway.org.uk> writes:
> 
>     John> If you have any comments or concerns, please raise them by
>     John> the end of Thursday 2009-03-25. I will then proceed with
>     John> generating the final release.

BTW, that should read Thursday 2009-03-26.

> During QA testing of an upcoming eCosPro release, Ross has found a
> problem in the C library's signal package. It is fairly simple to
> reproduce.
> 
>   ecosconfig new pc default   (any target using a recent compiler will do)
>   ecosconfig add fileio
>   enable CYGPKG_IO_SERIAL_TERMIOS_TERMIOS0
>   make tests
> 
> The build will fail with duplicate definitions of
> cyg_libc_signals_lock() and _unlock(). The problem is caused by this
> patch:
> 
>     2008-12-24  Andrew Lunn  <andrew.lunn@ascom.ch>
> 
> 	* include/signal.inl: (cyg_libc_signals_[un]lock): remove the
> 	static from these inline functions which are used by the none
> 	static inline signal() and raise().
>  
> The patch successfully eliminated some warnings but also changed the
> inlining semantics of those functions. The compiler is now generating
> real code for these functions, as well as inlining them, so if more
> than one module tries to use signal() or raise() then you'll get
> multiple definitions.
> 
> There is a simple quick fix:
> 
> Index: signal.inl
> ===================================================================
> RCS file: /cvs/ecos/ecos/packages/language/c/libc/signals/current/include/signal.inl,v
> retrieving revision 1.7
> diff -u -p -r1.7 signal.inl
> --- signal.inl	29 Jan 2009 17:49:52 -0000	1.7
> +++ signal.inl	25 Mar 2009 12:02:02 -0000
> @@ -102,7 +102,7 @@ extern void cyg_libc_signals_lock_do_unl
>  // cyg_libc_signals_lock() //
>  /////////////////////////////
>  
> -inline cyg_bool
> +extern __inline__ cyg_bool
>  cyg_libc_signals_lock(void)
>  {
>  #ifdef CYGSEM_LIBC_SIGNALS_THREAD_SAFE
> @@ -116,7 +116,7 @@ cyg_libc_signals_lock(void)
>  // cyg_libc_signals_unlock() //
>  ///////////////////////////////
>  
> -inline void
> +extern __inline__ void
>  cyg_libc_signals_unlock(void)
>  {
>  #ifdef CYGSEM_LIBC_SIGNALS_THREAD_SAFE
> 
> 
> This again prevents the compiler from generating real code for the
> functions, so the duplicate definitions go away. I think this is a
> sensible fix, but obviously it has not been tested. At this stage of
> the release process I'll wait for approval before checking it in.

[ snip ]

> John, Jifl: any objections to me checking in this fix?

Looking at the CVS logs, Andrew's change of 2008-12-24 was part of
series of check-ins targeting compiler warnings in general. None of the
other changes involved modifying inlined functions so I expect this is
an isolated regression.

From my perspective, it is clearly desirable to fix this for 3.0 but it
is indeed late in the release cycle so I would ask that you do perform a
basic run-time sanity test against the ecos-v3_0-branch before
committing it. Best to use one of the new pre-built eCos toolchains (GCC
4.3.2) for this.

Thanks

John Dallaway