Re: [PATCH] [BFD] Add support for reading msdos (MZ) executables.

[Zebediah Figura <z dot figura12 at gmail dot com>]

On 01/02/2018 10:20 AM, Nick Clifton wrote:
> Hi Zebediah,
> 
>>> This is awfully similar to the external_PEI_DOS_hdr structure defined in
>>> include/coff/pe.h.
> 
>> That's a good point; I'll move this into a common header. Should this be
>> done in a separate patch?
> 
> No, please include it in your revised patch when you post it.
> 
> 
>>>> +  /* Check that this isn't actually a PE, NE, or LE file.  */
>>>> +  if (bfd_seek (abfd, (file_ptr) H_GET_32 (abfd, hdr.e_lfanew), SEEK_SET) != 0
>>>> +      || bfd_bread (buffer, (bfd_size_type) 2, abfd) != 2)
>>>> +    {
>>>> +      if (bfd_get_error () == bfd_error_system_call)
>>>> +	return NULL;
>>>> +    }
>>>
>>> I think that you always want to return NULL here.  My guess is that you
>>> meant something like:
>>>
>>>          if (bfd_get_error () != bfd_error_system_call)
>>>            bfd_set_errror (bfd_error_wrong_format);
>>>          return NULL;
>>>
>>
>> I don't think so? For example, if the offset refers to an absurdly large
>> value (interpreted as such) then bfd_seek() will return
>> bfd_error_file_truncated, but we still want to treat it as an MZ file.
> 
> Ah - OK - in which case please could you add a comment there so that 
> simple folk like myself are not confused ? :-)  Just something like:
> 
>   /* An MZ file does not have to have a valid offset in the 
>      e_lfanew field, but if it does, then it must not point 
>      to a PE/NE/LE indicator.  */
> 
> 
> Cheers
>   Nick
> 
> 
> 
> 

Will do, thanks.