general comments on sysiinit patches

[Chris Johns]

On 2/02/2016 7:25 PM, Sebastian Huber wrote:
> 
> 
> On 29/01/16 07:28, Sebastian Huber wrote:
>>
>>
>> On 28/01/16 03:54, Gedare Bloom wrote:
>>> On Wed, Jan 27, 2016 at 9:29 AM, Sebastian Huber
>>> <sebastian.huber at embedded-brains.de>  wrote:
>>>> >
>>>> >
>>>> >On 27/01/16 15:01, Joel Sherrill wrote:
>>>>> >>
>>>>> >>
>>>>> >>On Tue, Jan 26, 2016 at 10:02 AM, Sebastian Huber
>>>>> >><sebastian.huber at embedded-brains.de
>>>>> >><mailto:sebastian.huber at embedded-brains.de>> wrote:
>>>>> >>
>>>>> >>     On 26/01/16 16:51, Joel Sherrill wrote:
>>>>> >>
>>>>> >>         Hi
>>>>> >>
>>>>> >>         I have questions that probably impact all/most of the
>>>>> patches
>>>>> >>         so thought I would start another thread. Then it is just
>>>>> >>         detailed review of the individual patches.
>>>>> >>
>>>>> >>         + We have used EXTERN to avoid duplicating extern and
>>>>> >>         instantiating data. You appear to have completely
>>>>> removed that
>>>>> >>         pattern with no discussion of changing the coding style.  I
>>>>> >>         really don't like duplication of the information.
>>>>> >>
>>>>> >>
>>>>> >>     There is no duplicate information. We have exactly one
>>>>> declaration
>>>>> >>     and one definition. The compiler checks that they harmonize.
>>>>> Its
>>>>> >>     necessary to move the definitions to the right module, so
>>>>> that the
>>>>> >>     linker can do its job and add the right initialization items.
>>>>> >>
>>>>> >>It was formerly one line that did both in a single location with
>>>>> >>documentation. The data was instantiated in a per manager file.
>>>>> The pattern
>>>>> >>was clear since it was used at least twenty times.
>>>> >
>>>> >
>>>> >A single line with some pre-processor magic. For SAPI and score its
>>>> a global
>>>> >switch.
>>>> >
>>>>> >>     This extern stuff is not mentioned here:
>>>>> >>
>>>>> >> https://devel.rtems.org/wiki/Developer/Coding/Conventions
>>>>> >>
>>>>> >>
>>>>> >>Come on.. citing letter of the "law" in this case is a pretty weak
>>>>> >>defense. There is a lot of stuff not mentioned there and you have
>>>>> been
>>>>> >>around long enough to know that that particular document has been
>>>>> grown a
>>>>> >>bit at a time specifically as we realized there is a pattern not
>>>>> covered.
>>>>> >>You and I both know that it is likely far from complete. And
>>>>> using "not
>>>>> >>mentioned" when there is clearly an existing pattern in the code
>>>>> itself is
>>>>> >>disingenuous.
>>>>> >>
>>>>> >>What a compiler may check and duplicating information are different
>>>>> >>things. You have changed the pattern in at least two ways:
>>>>> >>
>>>>> >>+ dedicated file for data instantiation per manager/handler
>>>>> >>+ single line to serve both as declaration and instantiation
>>>>> >>
>>>>> >>The two patterns have similar technical effect in that they
>>>>> declare and
>>>>> >>instance a variable. But you changed the pattern with no
>>>>> discussion about
>>>>> >>the pattern. That is the issue here.
>>>> >
>>>> >
>>>> >I didn't want to change a pattern. My goal was to implement the
>>>> linker set
>>>> >based initialization which I thought was consensus. For this its
>>>> important
>>>> >that a certain global variable definition and its initialization
>>>> >item/handler is in the same module. The mechanism is that the user
>>>> of a
>>>> >certain functional unit reference some global data of this
>>>> functional unit.
>>>> >This pulls in the initialization item/handler for this functional
>>>> unit. For
>>>> >the RTEMS managers this is usually quite easy, just place the
>>>> initialization
>>>> >item/handler to the definition of the objects information
>>>> definition. With
>>>> >this pre-processor magic to define global data you make it quite
>>>> hard to
>>>> >review this part. In how many other software projects is this
>>>> mechanism to
>>>> >define global data used? It confuses source code reference tools.
>>>> If you see
>>>> >this stuff the first time, how long do you need to understand what
>>>> is going
>>>> >on here? If you insist, then I can bring back this pattern, but I
>>>> don't
>>>> >think it makes things more clear.
>>>> >
>>> I spent > 1 hour staring at EXTERN the first time I encountered it,
>>> and still didn't understand until I asked, and was still a bit
>>> confused on the whole subject. What is the benefit to maintaining this
>>> pattern (Joel)?
>>>
>>> I'm good with changing the pattern, but I do agree that we ought to be
>>> clear this is a modification to how global variables have in the past
>>> primarily been declared/defined.
>>
>> How do we want to proceed? Chris, what is your opinion with respect to
>> this EXTERN magic?
>>
>> some.h
>>
>> #ifndef SOME_XYZ_EXTERN
>> #define SOME_XYZ_EXTERN extern
>> #endif
>>
>> SOME_XYZ_EXTERN type xyz;
>>
>> some_xyz.c
>>
>> #define SOME_XYZ_EXTERN
>> #include <some.h>
>>
>> vs.
>>
>> some.h
>>
>> extern type xyz;
>>
>> some_xyz.c
>>
>> #include <some.h>
>> type xyz;
>>
> 
> We have 2:1 against the EXTERN pattern, so I will commit the patches
> tomorrow if nobody objects in the meantime.
> 

I do not object to the patch or the change. The need to discuss the