PATCH: Check S-record with 0 size

[Alan Modra]

On Fri, Aug 29, 2014 at 08:06:28AM -0700, H.J. Lu wrote:
> On Thu, Aug 28, 2014 at 6:15 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Thu, Aug 28, 2014 at 08:25:34AM -0700, H.J. Lu wrote:
> >> Hi,
> >>
> >> I checked in this patch to fix S-record with 0 size reported at
> >>
> >> http://lists.gnu.org/archive/html/bug-binutils/2014-08/msg00110.html
> >
> > We should really be testing for other invalid byte counts.
> >
> >         * srec.c (srec_scan): Revert last change.  Report an error for
> >         S-records with less than the miniumum byte count.
> >
> 
> I don't think it is desirable.  For the given testcase, now we got
> 
> [hjl@gnu-6 binutils]$ ./strings /tmp/ohcrap.txt
> BFD: /tmp/ohcrap.txt:1: byte count 0 too small
> 
> S700

The thing is that srec_scan ought to provide this information in the
bulk of the file.  For example, if someone has been editing an
S-record file and gets something wrong, then it is good to know which
line.

On the other hand, srec_object_p shouldn't really be returning any
errors other than bfd_error_wrong_format, or fatal errors like
bfd_error_no_memory.  The difficulty is that srec_object_p does more
than just decide whether the bfd is an srec file;  It parses the
entire file.  In effect, any error results in "this isn't an srec
file".  So if we don't report S-record errors in srec_scan, we won't
have an opportunity to report them later.

I thought about fixing this by extracting "case 'S'" from srec_scan
into a separate function, untangling all the error reporting, then
using the new function in srec_object_p to verify the first line was a
valid S-record.  ie. in place of

  if (b[0] != 'S' || !ISHEX (b[1]) || !ISHEX (b[2]) || !ISHEX (b[3]))
    {
      bfd_set_error (bfd_error_wrong_format);
      return NULL;
    }

That seemed like too much work for little benefit.

-- 
Alan Modra
Australia Development Lab, IBM