| Message ID | 20190613070903.17214-9-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/17] binfmt_flat: remove flat_reloc_valid | expand |
On Thu, Jun 13, 2019 at 09:08:54AM +0200, Christoph Hellwig wrote: > Two branches of the ifdef maze actually have the same content, so merge > them. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > include/linux/flat.h | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/include/linux/flat.h b/include/linux/flat.h > index 2b7cda6e9c1b..19c586b74b99 100644 > --- a/include/linux/flat.h > +++ b/include/linux/flat.h > @@ -69,15 +69,13 @@ struct flat_hdr { > typedef union { > unsigned long value; > struct { > -# if defined(mc68000) && !defined(CONFIG_COLDFIRE) > +#if defined(__LITTLE_ENDIAN_BITFIELD) || \ > + (defined(mc68000) && !defined(CONFIG_COLDFIRE)) > signed long offset : 30; > unsigned long type : 2; > # elif defined(__BIG_ENDIAN_BITFIELD) > unsigned long type : 2; > signed long offset : 30; > -# elif defined(__LITTLE_ENDIAN_BITFIELD) > - signed long offset : 30; > - unsigned long type : 2; > # else > # error "Unknown bitfield order for flat files." > # endif > -- > 2.20.1 > FWIW, I wonder if keeping that type is worth bothering. Something like old_reloc(__be32 reloc) { u32 v = be32_to_cpu(reloc); int offset, type; #if (defined(mc68000) && !defined(CONFIG_COLDFIRE)) /* old m68k uses unusual format - type is in lower bits of octet 3 */ type = v % 4; offset = (int)v / 4; #else /* everything else (including coldfire) has it in upper bits of octet 0 */ type = v >> 30; offset = (int)(v << 2) >> 2; /* or (v & 0x1fffffff) - (v & 0x20000000) * 4 */ #endif ... and to hell with bitfields, aliasing unions, etc. Unless I'm misreading the whole thing, that is... Greg?
On 26/6/19 8:29 am, Al Viro wrote: > On Thu, Jun 13, 2019 at 09:08:54AM +0200, Christoph Hellwig wrote: >> Two branches of the ifdef maze actually have the same content, so merge >> them. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> >> --- >> include/linux/flat.h | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/include/linux/flat.h b/include/linux/flat.h >> index 2b7cda6e9c1b..19c586b74b99 100644 >> --- a/include/linux/flat.h >> +++ b/include/linux/flat.h >> @@ -69,15 +69,13 @@ struct flat_hdr { >> typedef union { >> unsigned long value; >> struct { >> -# if defined(mc68000) && !defined(CONFIG_COLDFIRE) >> +#if defined(__LITTLE_ENDIAN_BITFIELD) || \ >> + (defined(mc68000) && !defined(CONFIG_COLDFIRE)) >> signed long offset : 30; >> unsigned long type : 2; >> # elif defined(__BIG_ENDIAN_BITFIELD) >> unsigned long type : 2; >> signed long offset : 30; >> -# elif defined(__LITTLE_ENDIAN_BITFIELD) >> - signed long offset : 30; >> - unsigned long type : 2; >> # else >> # error "Unknown bitfield order for flat files." >> # endif >> -- >> 2.20.1 >> > > FWIW, I wonder if keeping that type is worth bothering. > Something like > old_reloc(__be32 reloc) > { > u32 v = be32_to_cpu(reloc); > int offset, type; > > #if (defined(mc68000) && !defined(CONFIG_COLDFIRE)) > /* old m68k uses unusual format - type is in lower bits of octet 3 */ > type = v % 4; > offset = (int)v / 4; > #else > /* everything else (including coldfire) has it in upper bits of octet 0 */ > type = v >> 30; > offset = (int)(v << 2) >> 2; /* or (v & 0x1fffffff) - (v & 0x20000000) * 4 */ > #endif > ... > > and to hell with bitfields, aliasing unions, etc. Unless I'm misreading > the whole thing, that is... Greg? I think you are right. This is much better. The old mc6800 is the odd one out, the rest have it in network order, and this makes that much clearer. Regards Greg
Hi Greg, On Wed, Jun 26, 2019 at 9:23 AM Greg Ungerer <gerg@linux-m68k.org> wrote: > On 26/6/19 8:29 am, Al Viro wrote: > > On Thu, Jun 13, 2019 at 09:08:54AM +0200, Christoph Hellwig wrote: > >> Two branches of the ifdef maze actually have the same content, so merge > >> them. > >> > >> Signed-off-by: Christoph Hellwig <hch@lst.de> > >> --- > >> include/linux/flat.h | 6 ++---- > >> 1 file changed, 2 insertions(+), 4 deletions(-) > >> > >> diff --git a/include/linux/flat.h b/include/linux/flat.h > >> index 2b7cda6e9c1b..19c586b74b99 100644 > >> --- a/include/linux/flat.h > >> +++ b/include/linux/flat.h > >> @@ -69,15 +69,13 @@ struct flat_hdr { > >> typedef union { > >> unsigned long value; > >> struct { > >> -# if defined(mc68000) && !defined(CONFIG_COLDFIRE) > >> +#if defined(__LITTLE_ENDIAN_BITFIELD) || \ > >> + (defined(mc68000) && !defined(CONFIG_COLDFIRE)) > >> signed long offset : 30; > >> unsigned long type : 2; > >> # elif defined(__BIG_ENDIAN_BITFIELD) > >> unsigned long type : 2; > >> signed long offset : 30; > >> -# elif defined(__LITTLE_ENDIAN_BITFIELD) > >> - signed long offset : 30; > >> - unsigned long type : 2; > >> # else > >> # error "Unknown bitfield order for flat files." > >> # endif > >> -- > >> 2.20.1 > >> > > > > FWIW, I wonder if keeping that type is worth bothering. > > Something like > > old_reloc(__be32 reloc) > > { > > u32 v = be32_to_cpu(reloc); > > int offset, type; > > > > #if (defined(mc68000) && !defined(CONFIG_COLDFIRE)) > > /* old m68k uses unusual format - type is in lower bits of octet 3 */ > > type = v % 4; > > offset = (int)v / 4; > > #else > > /* everything else (including coldfire) has it in upper bits of octet 0 */ > > type = v >> 30; > > offset = (int)(v << 2) >> 2; /* or (v & 0x1fffffff) - (v & 0x20000000) * 4 */ > > #endif > > ... > > > > and to hell with bitfields, aliasing unions, etc. Unless I'm misreading > > the whole thing, that is... Greg? > > I think you are right. This is much better. > The old mc6800 is the odd one out, the rest have it in network order, > and this makes that much clearer. Is that correct for Microblaze, which can be big or little endian? Gr{oetje,eeting}s, Geert
Hi Geert, On 26/6/19 6:18 pm, Geert Uytterhoeven wrote: > Hi Greg, > > On Wed, Jun 26, 2019 at 9:23 AM Greg Ungerer <gerg@linux-m68k.org> wrote: >> On 26/6/19 8:29 am, Al Viro wrote: >>> On Thu, Jun 13, 2019 at 09:08:54AM +0200, Christoph Hellwig wrote: >>>> Two branches of the ifdef maze actually have the same content, so merge >>>> them. >>>> >>>> Signed-off-by: Christoph Hellwig <hch@lst.de> >>>> --- >>>> include/linux/flat.h | 6 ++---- >>>> 1 file changed, 2 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/include/linux/flat.h b/include/linux/flat.h >>>> index 2b7cda6e9c1b..19c586b74b99 100644 >>>> --- a/include/linux/flat.h >>>> +++ b/include/linux/flat.h >>>> @@ -69,15 +69,13 @@ struct flat_hdr { >>>> typedef union { >>>> unsigned long value; >>>> struct { >>>> -# if defined(mc68000) && !defined(CONFIG_COLDFIRE) >>>> +#if defined(__LITTLE_ENDIAN_BITFIELD) || \ >>>> + (defined(mc68000) && !defined(CONFIG_COLDFIRE)) >>>> signed long offset : 30; >>>> unsigned long type : 2; >>>> # elif defined(__BIG_ENDIAN_BITFIELD) >>>> unsigned long type : 2; >>>> signed long offset : 30; >>>> -# elif defined(__LITTLE_ENDIAN_BITFIELD) >>>> - signed long offset : 30; >>>> - unsigned long type : 2; >>>> # else >>>> # error "Unknown bitfield order for flat files." >>>> # endif >>>> -- >>>> 2.20.1 >>>> >>> >>> FWIW, I wonder if keeping that type is worth bothering. >>> Something like >>> old_reloc(__be32 reloc) >>> { >>> u32 v = be32_to_cpu(reloc); >>> int offset, type; >>> >>> #if (defined(mc68000) && !defined(CONFIG_COLDFIRE)) >>> /* old m68k uses unusual format - type is in lower bits of octet 3 */ >>> type = v % 4; >>> offset = (int)v / 4; >>> #else >>> /* everything else (including coldfire) has it in upper bits of octet 0 */ >>> type = v >> 30; >>> offset = (int)(v << 2) >> 2; /* or (v & 0x1fffffff) - (v & 0x20000000) * 4 */ >>> #endif >>> ... >>> >>> and to hell with bitfields, aliasing unions, etc. Unless I'm misreading >>> the whole thing, that is... Greg? >> >> I think you are right. This is much better. >> The old mc6800 is the odd one out, the rest have it in network order, >> and this makes that much clearer. > > Is that correct for Microblaze, which can be big or little endian? It is true for all architectures that use flat. All fields inside a flat format binary are store in network order. The final processing of the relocation entries in the elf2flt converter tool: for (i=0; i<reloc_len; i++) reloc[i] = htonl(reloc[i]); Regards Greg
diff --git a/include/linux/flat.h b/include/linux/flat.h index 2b7cda6e9c1b..19c586b74b99 100644 --- a/include/linux/flat.h +++ b/include/linux/flat.h @@ -69,15 +69,13 @@ struct flat_hdr { typedef union { unsigned long value; struct { -# if defined(mc68000) && !defined(CONFIG_COLDFIRE) +#if defined(__LITTLE_ENDIAN_BITFIELD) || \ + (defined(mc68000) && !defined(CONFIG_COLDFIRE)) signed long offset : 30; unsigned long type : 2; # elif defined(__BIG_ENDIAN_BITFIELD) unsigned long type : 2; signed long offset : 30; -# elif defined(__LITTLE_ENDIAN_BITFIELD) - signed long offset : 30; - unsigned long type : 2; # else # error "Unknown bitfield order for flat files." # endif
Two branches of the ifdef maze actually have the same content, so merge them. Signed-off-by: Christoph Hellwig <hch@lst.de> --- include/linux/flat.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)