general comments on sysiinit patches

[Sebastian Huber]

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.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.