Re: [PRELIMINARY]: Patch to add bfd support for IBM s390

[Geoff Keating <geoffk at cygnus dot com>]

> Date: Thu, 24 Aug 2000 12:55:24 -0700
> From: Michael Snyder <msnyder@redhat.com>

> +/* additional s390/elf relocations */
> +  BFD_RELOC_390_8,
> +  BFD_RELOC_390_12,
> +  BFD_RELOC_390_16,
> +  BFD_RELOC_390_GOT12,
> +  BFD_RELOC_390_GOT32,
> +  BFD_RELOC_390_PLT32,
> +  BFD_RELOC_390_COPY,
> +  BFD_RELOC_390_GLOB_DAT,
> +  BFD_RELOC_390_JMP_SLOT,
> +  BFD_RELOC_390_RELATIVE,
> +  BFD_RELOC_390_GOTOFF,
> +  BFD_RELOC_390_GOTPC,
> +  BFD_RELOC_390_GOT16,
> +  BFD_RELOC_390_PC16DBL,
> +  BFD_RELOC_390_PLT16DBL,

Most of it seemed OK, although I didn't look at the s390-specific
files too closely.

I have some comments about this bit though:

- Comment.  Comments are full sentences, they start with an uppercase
  letter and end with a full stop and two spaces.  I know that
  this isn't always done, but it should be.  It also looks like best
  practise is to put a comment above each reloc saying what it does.

- What's the difference between BFD_RELOC_390_8 and BFD_RELOC_8?
  I suspect they're the same, in which case BFD_RELOC_8 should be
  used.  Likewise, although there is no BFD_RELOC_12, perhaps there
  should be.  Likewise, there's a BFD_RELOC_32_GOT_PCREL and
  BFD_RELOC_32_GOTOFF, one of which might be the same as 
  BFD_RELOC_390_GOT32.

  ...and perhaps if there was a BFD_RELOC_COPY we wouldn't need each
  port to define one... although that's now starting to look like hard
  work.  At least use the relocs that are already there, though.

-- 
- Geoffrey Keating <geoffk@cygnus.com>