general comments on sysiinit patches

[Joel Sherrill]

On Tue, Jan 26, 2016 at 10:02 AM, Sebastian Huber <
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.


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


>> + Do you have any size metrics for current --disable-posix on a BSP with
>> function sections enabled versus this new way?
>>
>
> No, I was not interested in this comparison since my goal is to always
> enable POSIX in the long run. The size will drop a bit. I used the SIS BSP
> for comparison since it uses the function/data sections for quite a while.
>
>
OK. Although not 100% accurate, comparing test sizes from 4.11 with POSIX
on and master with POSIX on should give an indication of the impact.  This
type of comparison has been more than adequate to show how other things
that impact code size have worked out. We know the core capabilities
required by each test haven't changed. What happened to its footprint
between two release points is an important measure.


>> + What impact does this have on BSP linkcmds? I am guessing since you
>> already added all the rtems sysinit magic, this will just work with no
>> issues. And we are stuck with missing function section KEEP's.
>>
>
> The linkcmds relevant patch sets are these and they are in place since
> September 2015:
>
>
> https://git.rtems.org/rtems/commit/?id=d0c39838146c6a186ddda3d95dac71c3fa90f11e
>
> https://git.rtems.org/rtems/log/?qt=range&q=b618d8cfc54f84d4ed03dc7b7fa510c872e6128a
>
>
>> That's what I thought. It was just a double check.


> Not opposed to doing the sysinit. I would just like to know what the size
>> delta is between the two solutions.  I am guessing this and function
>> sections will be consistently smaller as compared to when POSIX is
>> currently enabled.
>>
>> --joel
>>
>>
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>>
>
> --
> 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.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20160127/d2972fe7/attachment-0002.html>