Re: [patch #5524] Error file command line option

[Ben Pfaff]

John Darrington <address@hidden> writes:

> On Tue, Nov 07, 2006 at 07:05:35AM -0800, Ben Pfaff wrote:
>      John Darrington <address@hidden> writes:
>      
>      > I think that main.c is in need of a major overhaul, but I don't have
>      > the inclination to do that right now.
>      
>      What do you think needs to be done?
>
> 1.  parse_command_line should be closer to the start of main.  It's
>     where it is now, because it actually does more than parsing.  It
>     also performs actions on the data it parses.  Some of these
>     actions depend upon things being initialised, whereas others
>     cannot be initialised until the data has been parsed.  Hence we
>     have some of the _init functions called before parse_command_line,
>     and some in the body of the if {}.
>
>     I suggest that parse_command_line is changed to do just parsing,
>     and return the data it parses.  

OK.

> 2.  If terminate() is necessary, then for consistency there should
>     also be initiate().

terminate() is a function because it is called from two places.
We only initialize in one place.  But it wouldn't hurt.

> 3.  This code:
>
>     static bool terminating = false;
>     if (!terminating) 
>     {
>       terminating = true;
>
>     is racy.     Get rid of it, and use sigvec instead of signal, to
>     prevent re-entrancy.

OK.

> 4.  Probably a side issue:  I want to encapsulate the lexer, so that 
>     lex_init() and lex_done() are replaced by struct lexer
>     *create_lexer(void) and void destroy_lexer(struct lexer *)
>     respectively.  main would have to do something with the returned
>     lexer (I haven't thought what yet).

Yes, I have had the same goal for some time.

The global variables token, tokval, tokid, tokstr want to be
encapsulated in a structure too.  tokid and tokstr should
probably be merged somehow.  Some generalization and
orthogonalization (if that's a word) is warranted.

> 5.  Probably the same should also be done for outp_init/outp_done

OK.

I finally have a plan that I think will work for rewriting the
output subsystem, by the way.  I need to write up my thoughts.

> 6.  Ditto for getl_initialise. getl_initialise (from
>     src/language/linebuffer.c) needs to be rewritten.  Get rid of the
>     union and the enum type, and replace with an abstract definition.
>     This way we can have line buffers for say a gtk_entry widget or
>     some other thing that we haven't yet thought of.

OK.

> 7.  Where static data  is necessary, let's identify those which
>     are effectively const --- ie are set at startup, and remain with
>     these values until the program terminates.  Eg: config_path,
>     allow_internal_sort, program_name  etc.  There are ways of
>     enforcing this, but they're a bit cumbersome.

My favorite way is to make the interface a function that returns
the value in question, instead of using a variable.

allow_internal_sort isn't actually constant, and doesn't need to
be global.  Perhaps it should be put into sort_criteria or
otherwise made an argument to the sort functions.
-- 
Ben Pfaff 
email: address@hidden
web: http://benpfaff.org