Re: [PATCH] Fix configuration in assert.h.

[Sergei Organov <osv at javad dot com>]

Bart Veer <bartv@ecoscentric.com> writes:

>>>>>> "Sergei" == Sergei Organov <osv@javad.com> writes:
>
>     Sergei> Index: packages/isoinfra/current/ChangeLog
>     Sergei> ===================================================================
>     Sergei> RCS file: /var/local/group/firmware/cvsroot/ecos/packages/isoinfra/current/ChangeLog,v
>     Sergei> retrieving revision 1.1.1.1
>     Sergei> diff -u -r1.1.1.1 ChangeLog
>     Sergei> --- packages/isoinfra/current/ChangeLog	7 Dec 2005 12:06:35 -0000	1.1.1.1
>     Sergei> +++ packages/isoinfra/current/ChangeLog	7 Mar 2006 16:22:55 -0000
>     Sergei> @@ -1,3 +1,10 @@
>     Sergei> +2006-03-07  Sergei Organov  <osv@javad.com>
>     Sergei> +
>     Sergei> +	* include/assert.h: Replace #if defined(CYGINT_xxx) with #if
>     Sergei> +	CYGINT_xxx. CYGINT_xxx configuration variables are always
>     Sergei> +	defined to either 0 or 1, so checks of the former form are always
>     Sergei> +	evaluated to true.
>
> That is incorrect. CDL interfaces, which should normally be the only
> configury that uses CYGINT_, can have the "data", "bool", or
> "booldata" flavors with data being the default. If an interface has
> the "data" flavour then there will always be a #define with the value
> being the number of implementors of the interface (which can be > 1).
> If it has the "bool" flavor and there are no implementors then there
> will be no #define, otherwise there will be a #define for 1. If it has
> the "booldata" flavor and there are no implementors then there will be
> no #define, otherwise there will be a #define for the number of
> implementors.

OK, I must admit that when I failed to get rid of the first
implementation of assert(), I've only checked CYGINT_ISO_EXIT and it is
always defined. So it's indeed my mistake to assume all the CYGINT_xxx
have the same behavior.

>  
>     Sergei> Index: packages/isoinfra/current/include/assert.h
>     Sergei> ===================================================================
>     Sergei> RCS file: /var/local/group/firmware/cvsroot/ecos/packages/isoinfra/current/include/assert.h,v
>     Sergei> retrieving revision 1.1.1.1
>     Sergei> diff -u -r1.1.1.1 assert.h
>     Sergei> --- packages/isoinfra/current/include/assert.h	7 Dec 2005 12:06:35 -0000	1.1.1.1
>     Sergei> +++ packages/isoinfra/current/include/assert.h	7 Mar 2006 15:27:50 -0000
>     Sergei> @@ -73,7 +73,7 @@
>  
>     Sergei>  /* First preference is to be standards compliant */
>  
>     Sergei> -#if defined(CYGINT_ISO_STDIO_FORMATTED_IO) && defined(CYGINT_ISO_EXIT)
>     Sergei> +#if CYGINT_ISO_STDIO_FORMATTED_IO && CYGINT_ISO_EXIT
>  
> In this case CYGINT_ISO_STDIO_FORMATTED_IO has the booldata flavor so
> should be tested with #ifdef or #if defined(). However CYGINT_ISO_EXIT
> has the default flavour, i.e. "data", so testing with defined() serves
> no purpose. Hence I suspect the line should read:
>
>     #if defined(CYGINT_ISO_STDIO_FORMATTED_IO) && CYGINT_ISO_EXIT
>
> Alternatively the bug may be in the CDL with a missing flavor property
> for CYGINT_ISO_EXIT. This is jifl's code so I'll let him comment.

I'm sure CYGINT_ISO_EXIT is always defined. I didn't in fact check
CYGINT_ISO_STDIO_FORMATTED_IO. Anyway, if the line should read as you
suggest, I think there is something wrong about the way the
configuration variables are defined/checked. I think it's too
error-prone if one needs to look at corresponding CDL and then re-read
the above 10 lines of explanations just to correctly check for
absence/presence of a configurable feature.

So hopefully I've changed the wrong place and it's CYGINT_ISO_EXIT CDL
that must be fixed instead. ...checking...

Well, CYGINT_ISO_EXIT has the default ("data"?) flavor but so are a lot
of other CYGINT_xxx even in the same isoinfra.cdl, but
CYGINT_ISO_STDIO_FORMATTED_IO as well as many other has "booldata"
flavor.

Why the difference? Could somebody please explain, why, for example,

        cdl_interface CYGINT_ISO_ERRNO {
            display       "Number of implementations of errno variable"
            requires      { 1 >= CYGINT_ISO_ERRNO }
        }
    
        cdl_interface CYGINT_ISO_LOCALE {
            display       "Number of implementations of locale functions"
            requires      { 1 >= CYGINT_ISO_LOCALE }
        }

are "data" while, say,

        cdl_interface CYGINT_ISO_STDIO_FILEOPS {
            display       "Number of implementations of stdio file operations"
            flavor        booldata
            requires      { 1 >= CYGINT_ISO_STDIO_FILEOPS }
        }

        cdl_interface CYGINT_ISO_STRERROR {
            display       "Number of implementations of strerror() function"
            requires      { 1 >= CYGINT_ISO_STRERROR }
            flavor        booldata
        }

are "booldata"??? I'm deeply confused :(


-- Sergei.