Re: eCos uSTL port patch for review

[John Dallaway <john at dallaway dot org dot uk>]

Hi Uwe

Uwe Kindler wrote:

>> b) Most of the diff between this package and the master uSTL sources
>> concerns potentially undefined pre-processor macros. Clearly it's
>> preferable to modify the original sources as little as possible.
>> Defining certain macros to "0" in config.h brings its own dangers. Are
>> the changes you have made to the pre-processor configuration tests
>> acceptable upstream by the uSTL maintainer(s)?
> 
> Could you please explain what do you mean by "dangers". Do you mean
> undefining pre-processor macros (like #undef HAVE_INT64_T) could cause
> trouble to other eCos packages or to eCos build process?

My first thought was to use definitions such as:

   #define HAVE_INT64_T 0

so that "#if HAVE_INT64_T" still evaluates to false but the undefined
macro warning is eliminated. However, the danger is that someone might
try to use "#ifdef HAVE_INT64_T" and get an unexpected result. Perhaps
this is not such a big danger. Anyone see any big issues with this?

> Normally the
> config.h file is generated automatically by the bsconf tool but I
> provide an manually edited config.h file that is optimized for eCos
> support like it is done by penrillian for S60
> (http://www.penrillian.com/content/view/82/73/). Normally it should not
> be necessary to undefine a macro because if it exists, then we would
> like to use it and if it does not exist, then it is already undefined.
> So do you mean I should remove all #undefs?

I agree that the #undef seems unnecessary. To address both the above
concerns, the following seems best:

   #ifndef HAVE_INT64_T
   #define HAVE_INT64_T 0
   #endif

> Do you mean you would like to see the #ifdef __GXX_RTTI and #ifdef
> __EXCEPTIONS stuff in the uSTL mainline?

Not the eCos-specific changes. The uSTL maintainer(s) are unlikely to
accept these, but patches which allow silent compilation with "-Wundef"
should be acceptable and will reduce the size of the eCos uSTL patch
considerably.

> I will ask the maintainer if
> these changes are acceptable. Would this be a showstopper for import
> into eCos CVS if these changes are rejected?

Definitely not a showstopper. It's just a case of trying to minimise
effort for future re-imports of uSTL using whatever techniques are at
our disposal.

>> d) Are you (or anyone else) aware of any compatibility issues arising
>> from the switch to C99 vsnprintf() behaviour?
> 
> O.k., I understand that this change may be critical for existing eCos
> applications and needs intensive testing. To keep the risk to a minimum
> I have the following tree proosals:
> 
> 1)
> Make the C99 vfnprintf() behaviour an option. Disable this option by
> default. This would ensure that the existing and well tested behaviour
> would be used. If the uSTL package is pulled in, then this package would
> require the C99 implementation to get activated.
> 
> 2)
> Provide an additional C99 compliant vsnprintf() implementation in the
> uSTL package (i.e. from http://www.jhweiss.de/software/snprintf.html)
> that will be used by uSTL format functions. This would increase code
> size for final ecos application.
> 
> 3)
> Mix 1) and 2) - that means, by default and without uSTL the default
> vfnprinf() behaviour is active. If uSTL becomes active, then the new C99
> compliant vfnprintf() behviour becomes active. If the user prefers the
> default vfnprintf() behaviour if uSTL package is active, then the
> additional C99 compliant vsnprintf() implementation will be compiled
> into the eCos library.
> 
> What do you think about these proposals? I would like to go with 1.

I agree. Option 1 seems best. At some point in the future, C99
vsnprintf() behaviour can become the default.

Another issue regarding configurability is that this uSTL port currently
requires that many packages are loaded:

>     requires CYGPKG_IO
>     requires CYGPKG_ERROR
>     requires CYGPKG_MEMALLOC
>     requires CYGPKG_LIBM
>     requires CYGPKG_LIBC_STDLIB
>     requires CYGPKG_LIBC_STDIO
>     requires CYGPKG_LIBC_STRING
>     requires CYGPKG_LIBC_I18N
>     requires CYGPKG_LIBC_SIGNALS
>     requires CYGPKG_LIBC_STARTUP

The macros defined in config.h suggest that uSTL can adapt to having
various packages not present. Is it, therefore, possible to make eCos
uSTL more configurable? For example, if CYGPKG_MEMALLOC is not loaded,
would uSTL still be (partly) usable? If so, it would be good remove
"requires CYGPKG_MEMALLOC" in the CDL file and instead use the following
in config.h:

  #ifdef CYGPKG_MEMALLOC
  #define HAVE_MALLOC 1
  #else
  #define HAVE_MALLOC 0
  #endif

You may find that there are so many dependencies that this makes no
sense, but it is worth investigating.

John Dallaway