[Bug 1607] __RTEMS_SIZEOF_VOID_P__ flawed design

[bugzilla-daemon at rtems.org]

https://www.rtems.org/bugzilla/show_bug.cgi?id=1607

--- Comment #4 from Ralf Corsepius <ralf.corsepius at rtems.org> 2010-07-07 11:01:13 CDT ---
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > (In reply to comment #0)
> > > > This change is a massive design flaw:
> > > > 
> > > > > 2010-06-29 Joel Sherrill <joel.sherrill at oarcorp.com>
> > > > > 
> > > > > * configure.ac, score/include/rtems/score/percpu.h: Add
> > > > > __RTEMS_SIZEOF_VOID_P__ to cpuopts.h so percpu.h has this information
> > > > > available during build and after installation.
> > > > 
> > > > Sizes are compile time constants and therefore may depend upon compiler 
> > > > flags (e.g. multilibs) flags. Defining them in a globally exported 
> > > > header being shared by different architecture variants qualifies is absurd.
> > > > 
> > > > Proposal: Joel to revert his whole __RTEMS_SIZEOF_VOID_P__ patch-set.
> 
> It was the least intrusive solution at hand.
Wrong, this was an ugly hack.

> We have to have a per cpu
> data structure to support SMP and it has to have a few fields accessed from
> assembly language.
My view differs.

>  Please help find an acceptable way to support this.  
With all due respect, provided you knew in advance that what you did is an ugly
design flaw in advance (Even you said so, somewhere, either PM or in another
BZ) I find reverting your patches and to send you "back to the drawing board"
to be a reasonable response.

> > > Maybe we should add this define to a no-installation header file?
> > Right. #1 candidate would be config.h.
> 
> I would be OK with that EXCEPT that some (or all to follow your proposal
> below) of the interrupt processing code is outside the CPU Kit and needs to
> know this.
Correct, this reveils another piece of "questionable design" (lack of an
interface definion).

> > I.e. an alternative to reverting Joel's stuff would be to extract all defines
> > which are using __RTEMS_SIZEOF_VOID_P__ to a private per-BSP header and to
> > remove all use case from cpukit (Unfortunately, this doesn't seem to easily 
> > achievable).
> 
> That will require moving all IRQ processing code from the CPU Kit and moving it
> to libbsp somewhere. 
Correct ... direct consequence of your design.

> Plus adding some check in each BSP configure.ac.
Correct, however such checks can be centralized.

> > > Btw. is SIZEOF_CPU_CONTEXT also something suspicious in cpuopts.h?
> > Suspicious, yet. Whether it actually is depends. 
> > ATM, without having looked into the source-tree, I am a bit surprised it is
> > still around (I thought it was gone).
> 
> I don't think it is used but my grep only found it in an old ChangeLog:
> 
> $ grep -r SIZEOF_CPU_CONTEXT .
> ./cpukit/ChangeLog-pre2007:    Add SIZEOF_CPU_CONTEXT to cpuopts.h.
> 
> Where do you see it?  Did you already do something to remove it?
IIRC off-head, it was only used by the posix BSP and not by embedded BSP.

-- 
Configure bugmail: https://www.rtems.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.