general comments on sysiinit patches

[Sebastian Huber]

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;

-- 
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.