This is the mail archive of the
mailing list for the binutils project.
Re: PATCH: Check S-record with 0 size
- From: Alan Modra <amodra at gmail dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Sat, 30 Aug 2014 11:12:42 +0930
- Subject: Re: PATCH: Check S-record with 0 size
- Authentication-results: sourceware.org; auth=none
- References: <20140828152534 dot GA4435 at intel dot com> <20140829011514 dot GH15716 at bubble dot grove dot modra dot org> <CAMe9rOoJ=4gi+e6=cX-5phAW6sqGeYHEspdwNN-zoqZSQvvU9A at mail dot gmail dot com>
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 <email@example.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
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
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 != 'S' || !ISHEX (b) || !ISHEX (b) || !ISHEX (b))
That seemed like too much work for little benefit.
Australia Development Lab, IBM