[Fortran] PR 50109 - fix skipping multiple comment lines in namelists

The patch should be rather simple and self explaining.

Build and regtested on x86-64-linux.
OK for the trunk?

I wonder how far we should backport; the program is said to work in 
4.1.2 and 4.2.5, but to fail in 4.3.6 up to the trunk. Oldest maintained 
GCC is 4.4.6. The patch is relatively simple, but as the long time 
between breakage and report shows, it does not seem to affect many users.

I think we should backport the patch at least to 4.6 and 4.5 , but one 
could also consider 4.4 or even 4.3. What do you think?

Tobias

PS: RHES 6 uses 4.4, SLES uses 4.3 with 4.5 as technical preview.

On 08/18/2011 11:59 AM, Tobias Burnus wrote:
> The patch should be rather simple and self explaining.

I just realized that I forgot to add more context to the diff. Thus, 
below the patch itself with some more lines (copied from 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50109#c4).

Without, it looks less obvious than it actually is.

One simply jumps again to the top of the "do { } while()" after removing 
the "!" line. The LIBERROR_END check is superfluous as it is already 
tested when moving to the top of "do {".

I was thinking of using "continue" or goto next, but the former will 
still check the while() condition and I think the latter looks more 
complicated/cluttered than the "c='\n';".

Tobias

@@ -345,28 +345,19 @@ eat_separator (st_parameter_dt *dtp)
           do
             {
               if ((c = next_char (dtp)) == EOF)
                   return LIBERROR_END;
               if (c == '!')
                 {
                   err = eat_line (dtp);
                   if (err)
                     return err;
-                 if ((c = next_char (dtp)) == EOF)
-                   return LIBERROR_END;
-                 if (c == '!')
-                   {
-                     err = eat_line (dtp);
-                     if (err)
-                       return err;
-                     if ((c = next_char (dtp)) == EOF)
-                       return LIBERROR_END;
-                   }
+                 c = '\n';
                 }
             }
           while (c == '\n' || c == '\r' || c == ' ' || c == '\t');
           unget_char (dtp, c);

On Thursday 18 August 2011 11:59:36 Tobias Burnus wrote:
> The patch should be rather simple and self explaining.
It is indeed with the extra context.

> 
> Build and regtested on x86-64-linux.
> OK for the trunk?
Yes.

> 
> I wonder how far we should backport; the program is said to work in
> 4.1.2 and 4.2.5, but to fail in 4.3.6 up to the trunk. Oldest maintained
> GCC is 4.4.6. The patch is relatively simple, but as the long time
> between breakage and report shows, it does not seem to affect many users.
> 
> I think we should backport the patch at least to 4.6 and 4.5 ,
makes sense.
> but one could also consider 4.4 or even 4.3. What do you think?
It looks like a real bug, with a simple fix, so I would say 4.4 too.
About 4.3, as it is unmaintained, we are not supposed to backport patches for 
it?

Mikael

On Thu, Aug 18, 2011 at 03:37:44PM +0200, Mikael Morin wrote:
> On Thursday 18 August 2011 11:59:36 Tobias Burnus wrote:
> > 
> > I wonder how far we should backport; the program is said to work in
> > 4.1.2 and 4.2.5, but to fail in 4.3.6 up to the trunk. Oldest maintained
> > GCC is 4.4.6. The patch is relatively simple, but as the long time
> > between breakage and report shows, it does not seem to affect many users.
> > 
> > I think we should backport the patch at least to 4.6 and 4.5 ,
> > makes sense.
> > but one could also consider 4.4 or even 4.3. What do you think?
> It looks like a real bug, with a simple fix, so I would say 4.4 too.
> About 4.3, as it is unmaintained, we are not supposed to backport patches for 
> it?

I agree with Mikael that the patch looks fairly simple (so
a bad side effect should not occur).  A backport to 4.4 is
fine with me.