Re: PATCH (Fortran) Prevent Dereferencing Of Null Pointer In g77spec.c

[craig at jcb-sc dot com]

>  In message <37E67AC7.E8C2E966@moene.indiv.nluug.nl>you write:
>  > Rationale:
>  > 
>  > The pointer argv passed by reference to lang_specific_driver is returned
>  > with a possibly updated value - if the code found it necessary to
>  > restructure the command line.
>  > 
>  > However, g77spec.c's lang_specific_driver "forgets" to initialise the
>  > static global pointer that is to contain the return value.  Hence, if
>  > the command line never has to be restructured, a zero value is returned
>  > (all static variables are guaranteed to start off as zero).
>Agreed.  However, I'm still trying to figure out the rationale behind the
>structure of the code.  We actually have 3 argv pointers, the old one, the
>new one and the "real new" one.

The "real new" one was recently added by somebody else.  It looks like
just an attempt to work around `const' warnings or something.  It should
be used just for two lines of code (the first two ref'ing it), it should
have been declared within the innermost block containing those two lines,
and therefore the later (incorrect) reference to it would have been
caught as a bug.

So, now that I've looked at it, it seems to me the patch preserved
the additional complexity when it could have cut down on it.
I.e. it's that last reference to real_g77_newargv that should be
just g77_newargv.  Then, to avoid confusion in the future, the decl
for real_g77_newargv should be moved to that little block that does
the allocation, which would contain the only two references to it
(a def followed by a use).

This problem probably just came from first modifying another spec.c
file and then copying over the mods to g77spec.c without examining
the flow of control, etc.  Similar code, but not, AFAICT offhand,
the same bug, is found in g++spec.c.

>It almost looks like we were trying to avoid copying the arg list if there
>was nothing to do.  Which may seem to make sense until you realize that
>added a lot more complexity to the code :(  Given the fact this stuff is not
>on the critical path (unless you're running the testsuite) clarity should
>probably be emphasized over speed.

I believe I designed it that way only to try and copy the way g++spec.c
worked -- it doesn't always copy the arg list either, or at least it
didn't.

If you compare g++spec.c to g77spec.c, and consider what the latter is
trying to do, I suggest you'd find the latter is actually easier for
someone to understand.  Well, maybe not so much of a difference as
between the old g++.c and g77.c command drivers, but, I created the
g77.c driver by redesigning a copy of g++.c so it'd be easier to
maintain, and accommodate some extra flexibility in terms of adding
new options to the command line, etc.  I preserved the avoidance of
copying so people wouldn't say "but it's more of a memory hog than
g++.c for the simple cases".

Then I re-made g77spec.c by pouring some of the old g77.c into the
g77spec.c that somebody else had made to get EGCS 1.0 working,
which reintroduced what I had thought was a fairly simple design.

I don't mind someone cutting out the code to avoid always copying
the command-line arguments in g77spec.c, but would suggest they
do that in the other drivers too, as it might make them simpler as
well.

        tq vm, (burley)