Re: Always provide exit() & friends prototypes.

[Sergei Organov <osv at javad dot com>]

Jonathan Larmour <jifl@eCosCentric.com> writes:
> Sergei Organov wrote:
>> Jonathan Larmour <jifl@eCosCentric.com> writes:
>>> Sergei Organov wrote:
>>>> Hello,
>>>>
>>>> A short patch below ensures exit(), etc. prototypes are provided in
>>>> <stdlib.h> even when CYGINT_ISO_EXIT is undefined.
>>>>
>>>> For me it solves the problem that I don't wish to use libc_startup
>>>> package, but do use exit()/abort() routines in applications. So the
>>>> situation is that application itself provides implementation of those
>>>> functions, while their declarations are expected to be found in the
>>>> <stdlib.h>.
>>> Then you should add your own package and implement CYGINT_ISO_EXIT;
>>
>> Even if I can't think of an implementation? A package that provides
>> empty or CYG_FAIL'ing exit()? Why?
>
> Then fix your code not to use it :-), or "#define exit(x)" in an
> application header or something. I think it's much more valuable to
> more eCos users that people who call functions that are not included
> due to configuration choices that they have made get it picked up
> straight away at compile time.
>
>>> or declare them in your own application-specific header.
>>
>> Not an option. Almost any C code on this planet expects to find exit()
>> declaration in the <stdlib.h>.
>
> Only because they assume it is a conforming implementation, meaning
> there's a real implementation.

IMHO most of the code that calls exit(), calls it just to exit
application, whatever that actually means, and they only assume that
this call will never return back.

> What we're talking about here is a non-conforming implementation (due
> to deliberate configuration choices by the programmer).

Yes, sure, but what is your point? Are you saying that eCos provides
conforming implementation whenever exit() is declared in <stdlib.h>? 
What about the case when, say, CYGFUN_LIBC_ATEXIT is set to 0? Is eCos
implementation still conforming? If not, why exit() is still defined in
<stdlib.h> in this case? Moreover, declaration of atexit() that in this
case lacks definition, is still there as well.

Another logical trouble is why do you care about conformance of the
implementation when you even don't provide conforming <stdlib.h>, as
<stdlib.h> without exit() declaration is obviously non-conforming.

>>> Only OS definitions should go in <stdlib.h>.
>>
>> The contents of <stdlib.h> is specified by the C standard, and exit() is
>> standard declaration that should be there in <stdlib.h> according to the
>> C standard. Currently (at some configuration options) eCos provides
>> non-standard <stdlib.h> and non-standard libc. I'd vote for standard
>> <stdlib.h> and non-standard libc (missing exit() definition) instead,
>
> I vote no declaration if there is no definition, in order to pick up
> configuration misunderstandings as soon as possible.

Well, let's then think why there is no definition in the first place? 
eCos has the definition for exit(), and in fact it in no way depends on
existence of some thread called "main" that invokes "main()" routine, so
exit() existence should not depend on the presence/absence of the
libc_startup package. Therefore, provided you still insist
CYGINT_ISO_EXIT is somehow useful in its current meaning, "when not set,
eCos provides neither declarations nor definitions of exit() and
friends", could we please make the CYGINT_ISO_EXIT just cdl_option with
default value 1?

Or should we better create separate libc_exit package that will
"implement" CYGINT_ISO_EXIT (that could then be "required" by other
packages)?

Anyway, I still believe something is definitely wrong here in its
current state.


[...]

>>> That's a separate problem due to failing to take into account CDL
>>> dependencies. Fixed with the attached patch, checked in.
>>
>> No, that's exactly this problem, -- see below.
>>
>> [...]
>>> +#if CYGINT_ISO_EXIT
>>>      exit(1000 + sig); // FIXME
>>> +#else
>>> +    CYG_FAIL("Default signal handler called - no exit available");
>>> +#endif
>>
>> Sorry, but this looks more like a work-around. In C, if you need exit(),
>> you call exit(), and if you need to call exit(), your include <stlib.h>,
>> period. If you still don't see the problem, there are actually hundreds
>> places that call exit() in the eCos "packages/" tree (I've counted
>> roughly 230). Are you going to check/fix all those as well?! That is
>> exactly the problem that I faced with in applications.
>
> I did consider whether it was better or not to instead put in a
> "requires CYGINT_ISO_EXIT" in the package CDL, but I thought that
> would annoy you more so chose it like I did. I am happy to switch if
> you think that leads to a more consistent implementation - I think I'd
> agree.

Trying to look at things from your POV, I think that there should be
separate single libc_exit package that implements "CYGINT_ISO_EXIT", and
that other packages could then "require" it. Though from my POV, there
is absolutely no need for this complexity, -- eCos can easily always
provide its current exit() instead. What's wrong about the latter?

> Incidentally there is an eCos kernel thread class method called
> exit(), so I expect you have a lot of false positives in that count;

No, I've excluded (most) exit() class method calls. With them, the count
was at 260. But even if it's 100, or even 10, the problem remains, and
you didn't yet suggest any affordable solution.

> as well as of course those packages which do have consistent CDL
> "requires" statements.

Of course. Could you please tell the name of at least one package that
"requires" CYGINT_ISO_EXIT?

The less dependencies you have, -- the better, right? Dependencies that
are absolutely non-obvious are bad. Dependency on CYGINT_ISO_EXIT to be
able to call *standard* exit() function is absolutely non-obvious,
that's why nobody calling exit() would even think that he now depends on
something.

Worse yet, depending on CYGINT_ISO_EXIT currently implies dependency
on libc_startup, that is very confusing and IMHO is plain wrong.

>> I think the CYG_FAIL() you have added rather belongs to the exit()
>> implementation, provided you prefer run-time checks (that besides are
>> off by default) over link-time checks.
>
> No I don't prefer run-time checks, and preferably not even link-time
> checks, if compile-time checks are possible.

Yes, that's my preference as well. Unfortunately, it doesn't work well
with standard C functions, and the next option to take is link-time
error.

> In your specific case I do think having a header which does:
> #define exit(x)   /* nothing */
> (or defined to whatever you want) should do.

No. My case is not specific in any way. Please try to do what you suggest
for those ~200 occurrences of exit() in the eCos source tree first, then
I'll re-consider your suggestion.

In addition, please take into account that unlike eCos sources, it's not
an option in applications to depend on *non-standard* CYGINT_ISO_EXIT
define to be able to call *standard* function.

Maybe I'll try to factor-out exit() & friends into a separate
configuration entity if I have time, but for now I'm forced to stick to
my original patch, even though I prefer to minimize my local
modifications of the eCos tree, sorry.

-- 
Sergei.