This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Large array printing patch for Fortran


On Mon, Sep 15, 2003 at 08:50:58PM +0100, David Lecomber wrote:
> On Mon, Sep 15, 2003 at 11:58:35AM -0400, Daniel Jacobowitz wrote:
> >
> >Yes, I know that this patch hasn't been reviewed.  That's a shame.  Nag
> >the list if you get impatient, but please stop checking things in to
> >CVS!  You're listed as a "Write After Approval" maintainer.  If you
> >can't wait for approval I'll ask to have you removed from that list.
> >
> >This was on my list to look at this week, when we were sure 6.0 was in
> >the hole.
> 
> Hi Daniel,
> 
> You reviewed this one May 16th -- I made the typographical corrections
> you asked for.  It could be that gdb-6.0 doesn't suffer from the bug
> and the fix would be unnecessary (but nonetheless harmless) now.  If
> it doesn't work, please roll it back.
> 
> I asked another gdb maintainer if I'd got the hang of what the
> procedures were - after my naughty commit last week ;-) - and he
> confirmed I now understood it.  On not getting a response from the
> list, I took that to mean it's okay.  Well, actually he said 'wait for
> responses'; I timed-out after 5 days.  My mistake, mea culpa, but I
> wasn't given any indication that a reply would occur!

Sorry, but that's life when all the maintainers are busy :(  Just
please don't time-out into committing.

In any case:
  - you left the whitespace change before f77_print_array_1.
  - you still haven't gotten GNU coding style right:

+      if (*elts >= print_max && i < F77_DIM_SIZE (nss)) {
+        fprintf_filtered (stream, "...");
+      }

Should be either:
+      if (*elts >= print_max && i < F77_DIM_SIZE (nss))
+        {
+          fprintf_filtered (stream, "...");
+        }
or
+      if (*elts >= print_max && i < F77_DIM_SIZE (nss))
+        fprintf_filtered (stream, "...");

Probably the latter.

+      for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++, (*elts)++)
should be
+      for (i = 0; i < F77_DIM_SIZE (nss) && (*elts) < print_max;
+           i++, (*elts)++)

i.e. watch line length.


+         if ((( *elts) == print_max - 1) && (i != (F77_DIM_SIZE (nss) - 1)))
should be
+         if ((*elts == print_max - 1) && (i != F77_DIM_SIZE (nss) - 1))


That's why I asked you to repost first.  It saves us iterating inside
of CVS.

The patch itself is fine.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]