Re: [PATCH] Move common code to a new function

[Steve Kargl <sgk at troutmask dot apl dot washington dot edu>]

On Sun, Apr 15, 2007 at 11:06:40PM -0700, Brooks Moses wrote:
> Steve Kargl wrote:
> >The attached patch has been compiled and regression tested
> >on i386-*-freebsd.  It moves common code in primary.c and
> >io.c into a new function in match.c.  The code should be
> >self-explanatory.  Ok for mainline?
> 
> Looks reasonable to me, but I do have a question:
> 
> >@@ -142,39 +142,8 @@ next_char (int in_string)
> >   if (gfc_option.flag_backslash && c == '\\')
> >     {
> >       locus old_locus = gfc_current_locus;
> >-
> >-      switch (gfc_next_char_literal (1))
> [...]
> > 
> >       if (!(gfc_option.allow_std & GFC_STD_GNU) && !inhibit_warnings)
> > 	gfc_warning ("Extension: backslash character at %C");
> 
> Why are you not incorporating the whole "if (gfc_option.flag_backslash)" 
> structure, and gfc_current_locus handling in the case where it's an 
> unknown code, and the warning message, all into the function too?  From 
> here, it looks like it's also all the same code in both places, and I 
> would think it would be a good idea to place the whole structure into a 
> new gfc_match_special_char function that gets called regardless of the 
> state of flag_backslash, and does nothing if backslashes aren't supposed 
> to be expanded.

I'll change it.  It was my first attempt of reducing the code to
a single function.  I started by removing the simple switch
statement in primary.c, placing it into match.c, and then
adjusting io.c.

> Also, something that is likely to be just my ignorance:
> >+  match m;
> >+
> >+  m = MATCH_YES;
> Is there a reason this isn't "match m = MATCH_YES;"?

Personal style.  I've seen too much C code of the form
  
  char *s1, *s2;
  int i, j=0, k;
  float x;
  double d;

The initialize of j is buried in a declaration, and could be missed
when reading the code.  I prefer a clear demarcation between declaration
and initialization.  If I make the above suggested change, then
the initialization becomes moot.

-- 
Steve