Home Original page

[PATCH] bfd/elf-hppa.h: Cleanup constants and comments

Helge Deller deller@gmx.de
Tue Aug 21 21:19:00 GMT 2018 
Hi Nick,

On 21.08.2018 16:30, Nick Clifton wrote:
>> Minor cleanups. Please commit if Ok.
> 
> Actually I have a couple of questions:
> 
>>  #if ARCH_SIZE == 64
>> -      hdr->sh_type = SHT_LOPROC + 1;
>> +      hdr->sh_type = SHT_LOPROC + SHT_PROGBITS;
> 
> The sh_type field is a single value, not a bitfield, so constructing a value
> like this is wrong.  A better solution, I would suggest, is to use the already
> defined value for this particular field.  IE:
> 
>       hdr->sh_type = SHT_PARISC_UNWIND;

Agreed. Much better!
 
>>  #else
>> -      hdr->sh_type = 1;
>> +      hdr->sh_type = SHT_PROGBITS;
>>  #endif
> 
> This is OK.

Ok.

>> -      /* I have no idea if this is really necessary or what it means.  */
>> +      /* Size of one unwind table entry is 16 (=2^4) bytes.  */
>>        hdr->sh_entsize = 4;
> 
> But if the unwind table entries are 16 bytes long, when why isn't the
> sh_entry field set to 16 ?  I think that the comment should explain
> why 4 is the correct value (or else change the sh_entsize field to 16,
> if that is the correct thing to do).

I'm think that 4 is the correct value here, because I think I read that somewhere once.
In that case I think "2 left-shifted by this value" should show the number of bytes per table line. 
But looking at all other usages of sh_entsize, this seems unlikely and everything is still unclear.
So, maybe we should simply leave the comment as is, until someone finds out?

Updated patch attached.
 
Helge
-------------- next part --------------
diff --git a/bfd/elf-hppa.h b/bfd/elf-hppa.h
index 5eac12996d..1c0cde1ab3 100644
--- a/bfd/elf-hppa.h
+++ b/bfd/elf-hppa.h
@@ -1131,9 +1131,9 @@ elf_hppa_fake_sections (bfd *abfd, Elf_Internal_Shdr *hdr, asection *sec)
       asection *asec;
 
 #if ARCH_SIZE == 64
-      hdr->sh_type = SHT_LOPROC + 1;
+      hdr->sh_type = SHT_PARISC_UNWIND;
 #else
-      hdr->sh_type = 1;
+      hdr->sh_type = SHT_PROGBITS;
 #endif
       /* ?!? How are unwinds supposed to work for symbols in arbitrary
 	 sections?  Or what if we have multiple .text sections in a single
More information about the Binutils mailing list