On Sat, Aug 10, 2013 at 7:18 PM, Glenn Fowler <[email protected]> wrote:
>
> On Sat, 10 Aug 2013 18:56:30 +0200 Irek Szczesniak wrote:
>> On Sat, Aug 10, 2013 at 6:37 PM, Irek Szczesniak <[email protected]>
>> wrote:
>> > On Sat, Aug 10, 2013 at 7:46 AM, Glenn Fowler <[email protected]>
>> > wrote:
[snip]
>> >>> What I don't understand: Roland added a safeguard against malfunctions:
>> >>> /*
>> >>> * Make sure _GNU_SOURCE is active on Linux. Some versions
>> >>> * give us serious trouble in this case so we have this
>> >>> * assert to *abort* early instead of let us hunt for "ghost
>> >>> * bugs"
>> >>> */
>> >>> #ifdef __linux__
>> >>> #ifndef __USE_GNU
>> >>> #error "ASSERT: __USE_GNU should be defined by now"
>> >>> #endif
>> >>> #endif
>> >>
>> >>> This had been introduced to *prevent* this from happening. WTF was
>> >>> this assertion removed? This is really the kind of regression which
>> >>> should NEVER happen because it fucks up the rest of the applications.
>> >
>> > I have to apologies for the explicit language. Imagine being under
>> > pressure and then find a regression. A regression for which was
>> > considered impossible because multiple layers of safeguards had been
>> > established to make the AST build fail the hard way in case of a
>> > regression.
>> >
>> >>> Wasted time: 2h 17 minutes to find out that O_SEARCH was unavailable
>> >>> in our application. Anyone wanna pay?
>
>> FYI I was ranting about the removal of this #error in fcntl.c:
>> /*
>> * Make sure _GNU_SOURCE is active on Linux. Some versions
>> * give us serious trouble in this case so we have this
>> * assert to *abort* early instead of let us hunt for "ghost
>> * bugs"
>> */
>> #ifdef __linux__
>> #ifndef __USE_GNU
>> #error "ASSERT: __USE_GNU should be defined by now"
>> #endif
>> #endif
>
>> If #error is triggered then you have no __USE_GNU. Which results in
>> zero extensions like O_DIRECTORY or O_DIRECT, or on Linux >=3.1 no
>> O_PATH.
>
>> What was the reason for removing that #error? If its triggered then
>> you have problems and removing the #error doesn't remove the problems
>> for you.
>
> I don't like programming in build bombs -- its hard enough getting builds to
> work
Erm... in this case it's IMO better to bomb out early than letting
people search forever in other parts of AST.
Please remember this comment:
-- snip --
/*
* O_PATH is a Linux's variation of O_SEARCH. Since this is treated
* as extension it may not be available without _GNU_SOURCE.
* Even even with _GNU_SOURCE some Linux platforms have bugs in their
* headers which prevents the definition of O_PATH in some hideous
* cases. To prevent all this mess we just grab the octal value and
* define it as O_SEARCH.
* Do not change this. You'll end-up in hell, together with the Linux
* headers. Quickly. - rm
-- snip --
... I usually don't write such comments. This one was born out of very
very bad pain of figuring out why it happens. And that's why I added
the very fatal assert for |__USE_GNU| to ensure that if that logic
fails I don't have to search much further.
> I was concerned that some linux variants + FEATURE/standards might crash a
> build when
> O_PATH etc. are not supported
Erm... that can't happen because my original assert tests for
|__USE_GNU| and _not_ |O_PATH| ...
> I'd be willing to add a build triggerred regression report that
> lists missing/emulated functionality -- for starters it could be
> a small C program that #ifndef's critical macros and lists them at the end of
> the build
Grumpf... the issue is that over time such a report would get very
large and likely quickly ignored because it's not _fatal_ (first rule
of management with IT people: Noone reads reports unless the managers
do this with their staff at the meetings).
The regression that O_PATH doesn't work crept in several alpha
releases ago and ruined a couple of other things quite badly because
they aren't tested properly anymore (while I thought they are tested
because the assert guards against such mistakes from h*ll).
> #ifdef __linux__
> #if !O_SEARCH
> printf("package: panic: *** O_SEARCH should be defined in
> <ast_fcntl.h>\n");
> #endif
> #endif
Well... we need to figure out the Linux version and make that an
#error for Linux >= 3.0.0 ... otherwise it's IMO a waste of good code
space... ;-(
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) [email protected]
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 3992797
(;O/ \/ \O;)
_______________________________________________
ast-developers mailing list
[email protected]
http://lists.research.att.com/mailman/listinfo/ast-developers
