[RFA] cfgroot usability improvement

[RFA] cfgroot usability improvement

[Ben Elliston]

The following patch handles the case whereby foolish users (me!) run
sid on a binary file (foo.x) instead of its corresponding
configuration file (foo.x.conf).  If too many non-printable characters
are detected in the input file, sid will stop.

Okay to commit?


2001-05-21  Ben Elliston  <bje@redhat.com>

        * compConfig.cxx (cfgroot_component::num_invalid_chars): New.
        (cfgroot_component ctor): Initialise it.
        (cfgroot_component::consume_token): If num_invalid_chars exceeds
        5, emit an error message and stop.

Index: compConfig.cxx
===================================================================
RCS file: /cvs/cvsfiles/devo/sid/component/cfgroot/compConfig.cxx,v
retrieving revision 1.77
diff -u -c -r1.77 compConfig.cxx
*** compConfig.cxx	2001/03/26 19:18:38	1.77
--- compConfig.cxx	2001/05/21 04:09:15
***************
*** 107,112 ****
--- 107,113 ----
    unsigned line_num;
    bool parse(istream&);
    string current_token;
+   int num_invalid_chars;
    void consume_token(istream&);
    string next_token(istream&);
    bool parse_command(istream&);
***************
*** 255,260 ****
--- 256,266 ----
  	    {
  	      if (!isprint(ch))
  		{
+ 		  if (++num_invalid_chars > 5)
+ 		    {
+ 		      emit_error ("too many invalid characters--possibly a binary file?");
+ 		      exit (1);
+ 		    }
  		  emit_error(string("invalid character [") + 
  			     make_numeric_attribute(host_int_2((unsigned char) ch),
  						    ios::hex|ios::showbase) +
***************
*** 273,278 ****
--- 279,289 ----
  	}
        else // control code
  	{	
+ 	  if (++num_invalid_chars > 5)
+ 	    {
+ 	      emit_error ("too many invalid characters--possibly a binary file?");
+ 	      exit (1);
+ 	    }
  	  emit_error(string("invalid character [") + 
  		     make_numeric_attribute(host_int_2((unsigned char) ch),
  					    ios::hex|ios::showbase) +
***************
*** 545,551 ****
    stop_pin(this, & cfgroot_component::stop),
    verbose_p (false),
    autoprint_p (true),
!   persistent_p (false)
  {
    // suck in libtool preloaded symbols; must be called before lt_dlinit()
    LTDL_SET_PRELOADED_SYMBOLS ();
--- 556,563 ----
    stop_pin(this, & cfgroot_component::stop),
    verbose_p (false),
    autoprint_p (true),
!   persistent_p (false),
!   num_invalid_chars (0)
  {
    // suck in libtool preloaded symbols; must be called before lt_dlinit()
    LTDL_SET_PRELOADED_SYMBOLS ();

Re: [RFA] cfgroot usability improvement

[Frank Ch. Eigler]

Hi -

On Mon, May 21, 2001 at 02:11:15PM +1000, Ben Elliston wrote:
: [...]
: Okay to commit?

Good idea, except:


: + 		  if (++num_invalid_chars > 5)
: + 		    {
: + 		      emit_error ("too many invalid characters--possibly a binary file?");
: + 		      exit (1);
: + 		    }


cfgroot can't take it upon itself to *exit* the process.
Think of another way to abort the parsing process.


- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.5 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE7CRf2VZbdDOm/ZT0RAjWwAJ0UkzHixPAoXq+HB+xPUCZY3jeoMgCZAWB5
AzNZq7YMG3ByqhJIxaKRNMA=
=niyY
-----END PGP SIGNATURE-----

Re: [RFA] cfgroot usability improvement

[Ben Elliston]

>>>>> "Frank" == Frank Ch Eigler <fche@redhat.com> writes:

  Frank> Good idea, except:
  [...]
  Frank> cfgroot can't take it upon itself to *exit* the process.
  Frank> Think of another way to abort the parsing process.

Ah yes, of course.
How about this, then?  Tested, no regressions.

Ben


2001-05-22  Ben Elliston  <bje@redhat.com>

	* compConfig.cxx (cfgroot_component::num_invalid_chars): New.
	(cfgroot_component::configure): Initialise it.
	(cfgroot_component::max_invalid_chars): New enumerator.
	(cfgroot_component::emit_invalid_char_error): New method.
	(cfgroot_component::consume_token): Factor out common error
	handling code into cfgroot_component::emit_invalid_char_error.
	(cfgroot_component::parse): Stop parsing if too many invalid
	character codes are encountered and emit an error message.

Index: compConfig.cxx
===================================================================
RCS file: /cvs/cvsfiles/devo/sid/component/cfgroot/compConfig.cxx,v
retrieving revision 1.78
diff -u -c -r1.78 compConfig.cxx
*** compConfig.cxx	2001/05/22 03:55:41	1.78
--- compConfig.cxx	2001/05/22 08:45:04
***************
*** 1,7 ****
  // compConfig.cxx - The cfgroot component: configuration parsing, root
  // of component creation and management.  -*- C++ -*-
  
! // Copyright (C) 1999-2001 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
--- 1,7 ----
  // compConfig.cxx - The cfgroot component: configuration parsing, root
  // of component creation and management.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2001 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
***************
*** 107,112 ****
--- 107,113 ----
    unsigned line_num;
    bool parse(istream&);
    string current_token;
+   int num_invalid_chars;
    void consume_token(istream&);
    string next_token(istream&);
    bool parse_command(istream&);
***************
*** 153,162 ****
--- 154,169 ----
    bool autoprint_p;
    bool persistent_p;
  
+   // max number of invalid chars which may be in a config file
+   enum { max_invalid_chars = 12 };
+ 
    // a general purpose method for emitting error messages
    // FIXME: i18n
    void emit_error(const string& msg);
  
+   // emit an error for invalid character codes found in the input
+   void emit_invalid_char_error (const char ch);
+ 
    // track component name -> component object pointer
    typedef map<string,component*> component_map_t;
    component_map_t component_map;
***************
*** 185,190 ****
--- 192,207 ----
  }
  
  void
+ cfgroot_component::emit_invalid_char_error (const char ch)
+ {
+   if (++num_invalid_chars < max_invalid_chars)
+     emit_error(string("invalid character [") + 
+ 	       make_numeric_attribute(host_int_2((unsigned char) ch),
+ 				      ios::hex|ios::showbase) +
+ 	       string("] ignored."));
+ }
+ 
+ void
  cfgroot_component::consume_token(istream& input) // lexer
  {
    string buf;
***************
*** 254,265 ****
  	  do
  	    {
  	      if (!isprint(ch))
! 		{
! 		  emit_error(string("invalid character [") + 
! 			     make_numeric_attribute(host_int_2((unsigned char) ch),
! 						    ios::hex|ios::showbase) +
! 			     string("] ignored."));
! 		}
  	      else
  		buf += ch;
  
--- 271,277 ----
  	  do
  	    {
  	      if (!isprint(ch))
! 		emit_invalid_char_error (ch);
  	      else
  		buf += ch;
  
***************
*** 272,282 ****
  	  break; // end of token
  	}
        else // control code
! 	{	
! 	  emit_error(string("invalid character [") + 
! 		     make_numeric_attribute(host_int_2((unsigned char) ch),
! 					    ios::hex|ios::showbase) +
! 		     string("] ignored."));
  	  continue;
  	}
      }
--- 284,291 ----
  	  break; // end of token
  	}
        else // control code
! 	{
! 	  emit_invalid_char_error (ch);
  	  continue;
  	}
      }
***************
*** 461,466 ****
--- 470,482 ----
      {
        bool this_ok = this->parse_command(cfile);
        cumulative_ok = cumulative_ok && this_ok;
+ 
+       if (num_invalid_chars >= max_invalid_chars)
+ 	{
+ 	  emit_error ("too many invalid characters--possibly a binary file?");
+ 	  cumulative_ok = false;
+ 	  break;
+ 	}
      }
    return cumulative_ok;
  }
***************
*** 483,488 ****
--- 499,505 ----
    this->config_file_history += this->config_file;
    unsigned last_line_num = this->line_num;
    this->line_num = 1;
+   num_invalid_chars = 0;
  
    ifstream cfile(find_sid_data_file(this->config_file).c_str());
    if(! cfile.good())