Re: [PATCH] bfd: tweak SET_ARCH_MACH of aout-cris.c

[Gunther Nikl <gnikl@justmail.de>]

Hans-Peter Nilsson <hp@bitrange.com> wrote:
> On Thu, 7 May 2020, Hans-Peter Nilsson wrote:
> > On Thu, 7 May 2020, Gunther Nikl wrote:
> >
> > > Hans-Peter Nilsson <hp@bitrange.com>:
> > > >
> > > > On Thu, 7 May 2020, Gunther Nikl wrote:
> > > > >
> > > > > A last question if you don't mind: there is a comment in
> > > > > set_sizes in front of line setting the relocation entry size.
> > > > > Since you had to add a case of bfd_arch_cris to "NAME (aout,
> > > > > machine_type)" adding such a case to "NAME (aout,
> > > > > set_arch_mach)" to use the generic set_sizes does not sound
> > > > > that bad. At least for me the machine_type function is also
> > > > > about target-specific things.
> > > >
> > > > I didn't see a question there.  Were the comments unclear?
> > > > I think I understand them.
> > >
> > > The question is: why is adding the bfd_arch_cris case in "NAME
> > > (aout, machine_type)" ok,
> >
> > Where is that?  (If you mistype something, I likely cannot
> > autocorrect.)
> 
> Oh, in aoutx.h.  Please excuse my blindness, don't forget it's
> about 20-25 years since I browsed the a.out parts.

Don't worry. I know very well that this is old code. When reading the
submission mails for the CRIS port you wrote that the port was from the
1992 time frame. Thats pretty old by now :)

> > > but extending the reloc case in "NAME (aout,
> > > set_arch_mach)" is not?
> 
> (Also in aoutx.h)
> 
> I see.  One is a designated place where you're *expected* to add
> a blurb for your machine, the other is a random place not every
> port has to edit and where I found a cleaner way than to IMHO
> uglify generic code with another target special-case.  Sure, a
> hair-thin difference, but still.  About the same reason you
> don't #ifdef AOUT_CRIS all over generic code just because you
> need something to be different for your port, but instead either
> use an existing mechanism or add a mechanism, hook, or ehatever
> you call it, and use that.  That said, both places should
> perhaps be better replaced by something that was easier on your
> eyes.

Thank you for these insights. I agree that target specific settings
should be in a target file. In this case I would probably have chosen
the easy way and modified the generic code since it already had such
target dependent code. What I have learned while studying the sources:
most of the time fixes/improvements are done to generic code which all
users of tht code get for free. A copy in another file is easily
overlooked.

> That answer will probably go for most other "why did you do X
> back then" cases.  Also, sometimes the code could have looked
> different back then; some hook that should have been used didn't
> exist at the time or whatever.

Regards,
Gunther