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

[Nick Clifton <nickc at redhat dot com>]

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