[1/2] Fix size of dos_message type in internal_extra_pe_filehdr

Message ID 20191017192652.zqui6gcq7ciqnhnn@foghorn.codeweavers.com
State New
Headers show
Series
  • [1/2] Fix size of dos_message type in internal_extra_pe_filehdr
Related show

Commit Message

Andrew Eikum Oct. 17, 2019, 7:26 p.m.
As shown by other declarations in include/coff/, and its usage in
bfd/peXXigen.c, dos_message is clearly meant to be 64 bytes long. On
Linux 64-bit, long is 8 bytes, which means the dos_message field in
internal_extra_pe_filehdr would be 128 bytes long.

Signed-off-by: Andrew Eikum <aeikum@codeweavers.com>

---
 include/coff/internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.23.0

Comments

Andrew Eikum Oct. 28, 2019, 3:16 p.m. | #1
Hello,

Is there anyone available to review these patches? This problem is
affecting Wine users[1], so I'd love to get a solution into binutils.
Please let me know if there's anything more I can do to help get this
fixed.

[1] https://bugs.winehq.org/show_bug.cgi?id=47718

Thanks for your time,
Andrew

On Thu, Oct 17, 2019 at 02:26:53PM -0500, Andrew Eikum wrote:
> As shown by other declarations in include/coff/, and its usage in

> bfd/peXXigen.c, dos_message is clearly meant to be 64 bytes long. On

> Linux 64-bit, long is 8 bytes, which means the dos_message field in

> internal_extra_pe_filehdr would be 128 bytes long.

> 

> Signed-off-by: Andrew Eikum <aeikum@codeweavers.com>

> ---

>  include/coff/internal.h | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/include/coff/internal.h b/include/coff/internal.h

> index c87dc8abaf..6069af972c 100644

> --- a/include/coff/internal.h

> +++ b/include/coff/internal.h

> @@ -54,7 +54,7 @@ struct internal_extra_pe_filehdr

>    unsigned short e_oeminfo;	/* OEM information; e_oemid specific, 0x0 */

>    unsigned short e_res2[10];	/* Reserved words, all 0x0 */

>    bfd_vma  e_lfanew;		/* File address of new exe header, 0x80 */

> -  unsigned long dos_message[16]; /* text which always follows dos header */

> +  unsigned int dos_message[16]; /* text which always follows dos header */

>    bfd_vma  nt_signature;   	/* required NT signature, 0x4550 */

>  };

>  

> -- 

> 2.23.0

> 

>
Nick Clifton Oct. 29, 2019, 8:06 a.m. | #2
Hi Andrew,

> Is there anyone available to review these patches?


Sorry - I missed these.

>> -  unsigned long dos_message[16]; /* text which always follows dos header */

>> +  unsigned int dos_message[16]; /* text which always follows dos header */


Patch approved and applied.

I guess that strictly speaking if it is important that this field be
64 bytes long then it should be declared as "char[64]" (although even
that would not work on hosts with 32-bit chars).  But this would mean
reworking more of the code, and your patch is simple and clean, so I
have gone ahead and committed it.

Are there other patches that need review ?  If so, please feel free to
ping this list.

Cheers
  Nick

Patch

diff --git a/include/coff/internal.h b/include/coff/internal.h
index c87dc8abaf..6069af972c 100644
--- a/include/coff/internal.h
+++ b/include/coff/internal.h
@@ -54,7 +54,7 @@  struct internal_extra_pe_filehdr
   unsigned short e_oeminfo;	/* OEM information; e_oemid specific, 0x0 */
   unsigned short e_res2[10];	/* Reserved words, all 0x0 */
   bfd_vma  e_lfanew;		/* File address of new exe header, 0x80 */
-  unsigned long dos_message[16]; /* text which always follows dos header */
+  unsigned int dos_message[16]; /* text which always follows dos header */
   bfd_vma  nt_signature;   	/* required NT signature, 0x4550 */
 };