general comments on sysiinit patches

[Sebastian Huber]

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.

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