[patch] Add Logging Support for SID Components

[patch] Add Logging Support for SID Components

[Dave Brolley]

[-- Attachment #1: Type: text/plain, Size: 2103 bytes --]

Hi,

The attached patch adds logging support for sid components in 3 stages:

1) Add a new level to the sid component heirarchy

    o fixed_attribute_map_with_logging_component inherits 
fixed_attribute_map_component and provides the logging capability
    o An output pin, ulog_out_pin, is defined here through which the log 
data is output.
    o printf-like logging (my immediate need) is provided via the 'void 
log (level, const char *fmt, ...)' method
    o Attributes record the logging settings (see options below)
    o A nice future addition would be a 'log (level)' method which would 
return some kind of stream object.

2) Add capability to sid-io-stdio (sid/consoles/stdio.cxx) for writing 
output to a file.

    o This provides logging to stdout or to a via via the component's 
ulog_out_pin

3) Expand the command line interface for sid

    o 3 new options control logging behavior
        --ulog-level=INTEGER
        --ulog-mode={less|match|equal}
        --ulog-file=FILENAME
    o As with many options, these relate to the previous --board 
specified or provide settings for all boards if specified before the 
first --board option.
    o The board level settings are kept in BoardCfg 
(sid/main/dynamic/commonCfg.h) by inhereting a new UlogCfg
    o The global level settings are kept in SessionCfg also by inherting 
from UlogCfg
    o Configuration may then use this info to set attributes and connect 
pins in the desired components to activate the logging feature.

NOTES:
o I needed to use vsnprintf or vasprintf if available and so I added 
checks for them in sid/component/configure.in
o I needed to define _ISOC99_SOURCE for vsnprintf or _GNU_SOURCE for 
vasprintf and so we now also have to define _BSD_SOURCE and _SVID_SOURCE 
which are normally defined by default if no _XXXXX_SOURCE is defined.
o I included the regenerated sid/component/config.h in the patch so you 
can see the new macros.

I'm requesting review of this patch, especially the placement of the new 
functionality in the component heirarchy and the configury changes to 
sid/component.

Thanks,
Dave

[-- Attachment #2: ulog.patch.txt --]
[-- Type: text/plain, Size: 23707 bytes --]

Index: sid/component/config.in
===================================================================
RCS file: /cvs/src/src/sid/component/config.in,v
retrieving revision 1.5
diff -c -p -r1.5 config.in
*** sid/component/config.in	11 Nov 2002 22:28:26 -0000	1.5
--- sid/component/config.in	30 May 2003 20:01:04 -0000
***************
*** 36,41 ****
--- 36,47 ----
  /* Define if you have the usleep function.  */
  #undef HAVE_USLEEP
  
+ /* Define if you have the vasprintf function.  */
+ #undef HAVE_VASPRINTF
+ 
+ /* Define if you have the vsnprintf function.  */
+ #undef HAVE_VSNPRINTF
+ 
  /* Define if you have the <../ucbinclude/sys/ioctl.h> header file.  */
  #undef HAVE____UCBINCLUDE_SYS_IOCTL_H
  
***************
*** 101,106 ****
--- 107,124 ----
  
  /* Define if curses library is usable */
  #undef HAVE_CURSES_LIBRARY
+ 
+ /* Define if ISO C99 Features in use */
+ #undef _ISOC99_SOURCE
+ 
+ /* Define if GNU extensions in use */
+ #undef _GNU_SOURCE
+ 
+ /* Define if SVID extensions in use */
+ #undef _SVID_SOURCE
+ 
+ /* Define if SVID extensions in use */
+ #undef _BSD_SOURCE
  
  /* Define if ftime(3) is declared via sys/timeb.h */
  #undef HAVE_FTIME_DECL
Index: sid/component/configure.in
===================================================================
RCS file: /cvs/src/src/sid/component/configure.in,v
retrieving revision 1.10
diff -c -p -r1.10 configure.in
*** sid/component/configure.in	6 May 2003 17:30:58 -0000	1.10
--- sid/component/configure.in	30 May 2003 20:01:05 -0000
*************** AC_CHECK_LIB(nsl, gethostbyname, socket_
*** 43,50 ****
  		socket_libs="$socket_libs -lresolv"))
  AC_SUBST(socket_libs)
  
! AC_CHECK_FUNCS(ftime times gettimeofday usleep strtoul strtoull select sched_yield)
  
  old_libs="$LIBS"
  LIBS="$LIBS $socket_libs"
  AC_CHECK_FUNCS(inet_aton inet_addr)
--- 43,61 ----
  		socket_libs="$socket_libs -lresolv"))
  AC_SUBST(socket_libs)
  
! AC_CHECK_FUNCS(ftime times gettimeofday usleep strtoul strtoull select sched_yield vsnprintf vasprintf)
  
+ dnl We want vsnprintf if available, otherwise try vasprintf
+ AC_CHECK_FUNC(vsnprintf,
+    [AC_DEFINE(_ISOC99_SOURCE, 1, [Define if ISO C99 Features in use])],
+    [AC_CHECK_FUNC(vasprintf,
+       [AC_DEFINE(_GNU_SOURCE, 1, [Define if GNU extensions in use])])])
+ 
+ dnl To pick up __USE_MISC stuff like inet_aton and putenv
+ AC_DEFINE(_SVID_SOURCE, 1, [Define if SVID extensions in use])
+ dnl To pick up __USE_BSD stuff like usleep
+ AC_DEFINE(_BSD_SOURCE, 1, [Define if SVID extensions in use])
+   
  old_libs="$LIBS"
  LIBS="$LIBS $socket_libs"
  AC_CHECK_FUNCS(inet_aton inet_addr)
Index: sid/component/consoles/components.h
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/components.h,v
retrieving revision 1.3
diff -c -p -r1.3 components.h
*** sid/component/consoles/components.h	3 Aug 2001 06:02:43 -0000	1.3
--- sid/component/consoles/components.h	30 May 2003 20:01:06 -0000
***************
*** 1,7 ****
  // file.cxx - Joint header file for nearby component classes.
  // -*- C++ -*-
  
! // Copyright (C) 1999, 2000 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 ----
  // file.cxx - Joint header file for nearby component classes.
  // -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
*************** class stdioConsole: public virtual compo
*** 97,106 ****
--- 97,110 ----
  {
  public:
    stdioConsole();
+  ~stdioConsole () throw();
  
  private:
+   void filename_attribute_set ();
+ 
    void write(host_int_4 value);
    void read(host_int_4);
+   void closeOutStream ();
  
    callback_pin<stdioConsole> activity_pin;
    callback_pin<stdioConsole> stdout_pin;
*************** private:
*** 110,115 ****
--- 114,121 ----
    string save_state ( );
    component::status restore_state (const string& state);
  
+   string out_filename;
+   ostream *out_stream;
  };
  
  
Index: sid/component/consoles/stdio.cxx
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/stdio.cxx,v
retrieving revision 1.2
diff -c -p -r1.2 stdio.cxx
*** sid/component/consoles/stdio.cxx	17 Jan 2001 21:05:09 -0000	1.2
--- sid/component/consoles/stdio.cxx	30 May 2003 20:01:06 -0000
***************
*** 1,10 ****
  // stdio.cxx - A simple console that uses standard I/O for
  // enunciation.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
  #include "config.h"
  #include "components.h"
  
--- 1,11 ----
  // stdio.cxx - A simple console that uses standard I/O for
  // enunciation.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
+ #include <fstream>
  #include "config.h"
  #include "components.h"
  
*************** namespace console_stuff
*** 13,32 ****
  
  stdioConsole::stdioConsole()
    :activity_pin(this, & stdioConsole::read),
!    stdout_pin(this, & stdioConsole::write)
  {
    add_pin("poll", &activity_pin);
    add_pin("stdin", &stdin_pin);
    add_pin("stdout", &stdout_pin);
    add_attribute_virtual ("state-snapshot", this,
  			 & stdioConsole::save_state,
  			 & stdioConsole::restore_state);
  }
  
  void
  stdioConsole::write(host_int_4 value)
  {
!   cout << static_cast<char> (value) << flush;
  }
  
  void
--- 14,58 ----
  
  stdioConsole::stdioConsole()
    :activity_pin(this, & stdioConsole::read),
!    stdout_pin(this, & stdioConsole::write),
!    out_filename ("*"),
!    out_stream (&cout)
  {
    add_pin("poll", &activity_pin);
    add_pin("stdin", &stdin_pin);
    add_pin("stdout", &stdout_pin);
+   add_attribute_notify ("filename", & out_filename, this, & stdioConsole::filename_attribute_set,
+ 			"output file setting");
    add_attribute_virtual ("state-snapshot", this,
  			 & stdioConsole::save_state,
  			 & stdioConsole::restore_state);
  }
  
+ stdioConsole::~stdioConsole () throw ()
+ {
+   closeOutStream ();
+ }
+ 
+ void
+ stdioConsole::closeOutStream ()
+ {
+   if (out_stream && out_stream != &cout)
+     delete out_stream;
+ }
+ 
+ void
+ stdioConsole::filename_attribute_set ()
+ {
+   // Associate the out_stream with the new filename
+   closeOutStream ();
+   out_stream = new ofstream (out_filename.c_str ());
+ }
+ 
  void
  stdioConsole::write(host_int_4 value)
  {
!   if (out_stream)
!     (*out_stream) << static_cast<char> (value) << flush;
  }
  
  void
Index: sid/include/sidattrutil.h
===================================================================
RCS file: /cvs/src/src/sid/include/sidattrutil.h,v
retrieving revision 1.3
diff -c -p -r1.3 sidattrutil.h
*** sid/include/sidattrutil.h	22 Nov 2002 20:35:00 -0000	1.3
--- sid/include/sidattrutil.h	30 May 2003 20:01:07 -0000
***************
*** 2,8 ****
  // mappings between application objects and their string
  // representations.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2002 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
--- 2,8 ----
  // mappings between application objects and their string
  // representations.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2002, 2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
***************
*** 10,15 ****
--- 10,16 ----
  #define SIDATTRUTIL_H
  
  #include <sidconfig.h>
+ #include <sidpinutil.h>
  #include "sidtypes.h"
  
  #include <string>
***************
*** 26,35 ****
--- 27,39 ----
  
  
  #include <limits.h>
+ #include <stdio.h>
+ #include <stdarg.h>
  
  // <ext/hash_map>? <hash_map>?  std::?  __gnu_cxx::?  Too much hassle.
  #undef HAVE_HASHING
  
+ #include <iostream>
  #if HAVE_SSTREAM
  #include <sstream>
  #elif HAVE_STRSTREAM_H
*************** make_attribute (const sid::any_int<IntTy
*** 1010,1015 ****
--- 1014,1145 ----
  	    else
  	      return ((*it).second)->parse_attribute (value);
        }
+   };
+ 
+   // A mix-in for classes with user logging.
+   class fixed_attribute_map_with_logging_component
+   : public virtual fixed_attribute_map_component,
+     public virtual fixed_pin_map_component
+   {
+   protected:
+     fixed_attribute_map_with_logging_component () :
+       ulog_level (0),
+       ulog_mode ("less"),
+       ulog_out_pin (),
+       buffer_size (4096), // big enough for now
+       saved_messages (),
+       saved_levels ()
+       {
+ 	add_attribute ("ulog-level", &ulog_level, "setting");
+ 	add_attribute ("ulog-mode",  &ulog_mode,  "setting");
+ 	add_pin ("ulog-out", & ulog_out_pin);
+ #if HAVE_VSNPRINTF || ! HAVE_VASPRINTF
+ 	buffer = new char[buffer_size];
+ #endif
+       }
+     ~fixed_attribute_map_with_logging_component () /* throw() */
+       {
+ 	// Output any saved messages.
+ 	output_saved_messages ();
+ #if HAVE_VSNPRINTF || ! HAVE_VASPRINTF
+ 	delete [] buffer;
+ #endif
+       }
+ 
+     virtual void log (sid::host_int_4 level, const char *fmt, ...)
+       {
+ 	if (ulog_out_pin.connections ().size () != 0)
+ 	  {
+ 	    // Output any saved messages first
+ 	    output_saved_messages ();
+ 
+ 	    // Check the logging level and mode.
+ 	    if (! check_level (level))
+ 	      return;
+ 	  }
+ 
+ 	// Write the message into a buffer.
+ 	int length;
+ 	for (;;)
+ 	  {
+ 	    va_list ap;
+ 	    va_start (ap, fmt);
+ #if HAVE_VSNPRINTF
+ 	    length = STDCTYPE(vsnprintf) (buffer, buffer_size, fmt, ap);
+ 	    va_end (ap);
+ 	    if (length < buffer_size)
+ 	      break;
+ 	    delete [] buffer;
+ 	    buffer_size = length + 256;
+ 	    buffer = new char[buffer_size];
+ #elif HAVE_VASPRINTF
+ 	    length = STDCTYPE(vasprintf) (&buffer, fmt, ap);
+ 	    va_end (ap);
+ 	    break;
+ #else
+ 	    length = STDCTYPE(vsprintf) (buffer, fmt, ap);
+ 	    va_end (ap);
+ 	    if (length >= buffer_size)
+ 	      std::cerr << "Error: ulog buffer overflow!!!" << std::endl;
+ 	    break;
+ #endif
+ 	  }
+ 
+ 	// If the output pin is not connected yet, Save the message for
+ 	// later. This happens when the log message is issued from the
+ 	// component's constructor.
+ 	if (ulog_out_pin.connections ().size () == 0)
+ 	  {
+ 	    saved_messages.push_back (std::string (buffer));
+ 	    saved_levels.push_back (level);
+ 	  }
+ 	else
+ 	  {
+ 	    // Otherwise, output the new message.
+ 	    for (int i = 0; i < length; ++i)
+ 	      ulog_out_pin.drive (buffer[i]);
+ 	  }
+ 
+ #if ! HAVE_VSNPRINTF && HAVE_VASPRINTF
+ 	free (buffer);
+ #endif
+       }
+ 
+ private:
+     bool check_level (sid::host_int_4 level)
+       {
+ 	if (level > ulog_level)
+ 	  return false;
+ 
+ 	if (level != ulog_level
+ 	    && (ulog_mode == "match" || ulog_mode == "equal"))
+ 	  return false;
+ 
+ 	return true;
+       }
+ 
+     void output_saved_messages ()
+       {
+ 	while (saved_messages.size () > 0)
+ 	  {
+ 	    if (check_level (saved_levels[0]))
+ 	      {
+ 		std::string s = saved_messages[0];
+ 		for (int i = 0; i < s.size (); ++i)
+ 		  ulog_out_pin.drive (s[i]);
+ 	      }
+ 	    saved_messages.erase (saved_messages.begin ());
+ 	    saved_levels.erase (saved_levels.begin ());
+ 	  }
+       }
+ 
+     sid::host_int_4 ulog_level;
+     std::string ulog_mode;
+     sidutil::output_pin ulog_out_pin;
+     char *buffer;
+     long buffer_size;
+     std::vector<std::string> saved_messages;
+     std::vector<sid::host_int_4> saved_levels;
    };
  }
  
Index: sid/main/dynamic/commonCfg.cxx
===================================================================
RCS file: /cvs/src/src/sid/main/dynamic/commonCfg.cxx,v
retrieving revision 1.3
diff -c -p -r1.3 commonCfg.cxx
*** sid/main/dynamic/commonCfg.cxx	16 Apr 2003 22:01:07 -0000	1.3
--- sid/main/dynamic/commonCfg.cxx	30 May 2003 20:01:07 -0000
*************** SessionCfg::SessionCfg (const string nam
*** 545,550 ****
--- 545,573 ----
    conn_pin (main_obj, "stopping", shutdown_seq, "input");
    yield_net->add_output (0, host_sched, "yield");
    init_seq->add_output (0, reset_net, "input");
+ 
+   AtomicCfg *ulog = new AtomicCfg ("ulog-*",
+ 				   "libconsoles.la", 
+ 				   "console_component_library", 
+ 				   "sid-io-stdio");
+   ulog_map["*"] = ulog;
+   add_child (ulog);
+ }
+ 
+ void
+ SessionCfg::add_ulog_file (const string name)
+ {
+   if (ulog_map.find (name) != ulog_map.end ())
+     return; // already there
+ 
+   // There is no existing logger for this file, so add a new one.
+   AtomicCfg *ulog = new AtomicCfg ("ulog-" + name, 
+ 				   "libconsoles.la", 
+ 				   "console_component_library", 
+ 				   "sid-io-stdio");
+   set (ulog, "filename", name);
+   ulog_map[name] = ulog;
+   add_child (ulog);
  }
  
  void SessionCfg::use_no_stdio ()
*************** BoardCfg::BoardCfg (const string name, 
*** 830,835 ****
--- 853,859 ----
  		    bool with_cpu_main_mem_connect) :
    ComponentCfg (name),
    AggregateCfg (name),
+   UlogCfg (),
    cache_flush_net (NULL),
    z_packet (with_z_packet),
    sess (s),
Index: sid/main/dynamic/commonCfg.h
===================================================================
RCS file: /cvs/src/src/sid/main/dynamic/commonCfg.h,v
retrieving revision 1.1
diff -c -p -r1.1 commonCfg.h
*** sid/main/dynamic/commonCfg.h	9 Jan 2003 04:22:50 -0000	1.1
--- sid/main/dynamic/commonCfg.h	30 May 2003 20:01:07 -0000
*************** class GlueSeqCfg : 
*** 166,175 ****
    int n;
  };
  
  // you should really only make one of these, with an empty name,
  // unless you want some crazy multi-session support.
  struct SessionCfg :
!   virtual public AggregateCfg
  {
    SessionCfg (const string name);
    virtual ~SessionCfg ();  
--- 166,189 ----
    int n;
  };
  
+ // Configs which support logging should inherit this
+ struct UlogCfg
+ {
+   UlogCfg (sid::host_int_4 l = 0, const string m = "less", const string f = "*")
+     : ulog_level (l), ulog_mode (m), ulog_file (f)
+   {}
+   virtual void set_ulog_level (sid::host_int_4 l) { ulog_level = l; }
+   virtual void set_ulog_mode (const string m) { ulog_mode = m; }
+   virtual void set_ulog_file (const string f) { ulog_file = f; }
+   sid::host_int_4 ulog_level;
+   string ulog_mode;
+   string ulog_file;
+ };
+ 
  // you should really only make one of these, with an empty name,
  // unless you want some crazy multi-session support.
  struct SessionCfg :
!   virtual public AggregateCfg, public UlogCfg
  {
    SessionCfg (const string name);
    virtual ~SessionCfg ();  
*************** struct SessionCfg :
*** 194,199 ****
--- 208,215 ----
    AtomicCfg *tcl_bridge;
    bool verbose;
    bool use_stdio;
+   void add_ulog_file (const string filename);
+   map<const string, AtomicCfg *> ulog_map;
  };
  
  class CpuCfg :
*************** class GdbCfg :
*** 278,284 ****
  };
  
  class BoardCfg :
!   virtual public AggregateCfg
  {
  public:
    BoardCfg (const string name,
--- 294,300 ----
  };
  
  class BoardCfg :
! virtual public AggregateCfg, public UlogCfg
  {
  public:
    BoardCfg (const string name,
*************** public:
*** 307,312 ****
--- 323,329 ----
    virtual void trace_disassemble ();
    virtual void trace_core ();
    virtual void write_config (Writer &w);
+ 
    virtual ~BoardCfg ();
  
    GlueSeqCfg *cache_flush_net;
Index: sid/main/dynamic/mainDynamic.cxx
===================================================================
RCS file: /cvs/src/src/sid/main/dynamic/mainDynamic.cxx,v
retrieving revision 1.3
diff -c -p -r1.3 mainDynamic.cxx
*** sid/main/dynamic/mainDynamic.cxx	9 Jan 2003 04:22:50 -0000	1.3
--- sid/main/dynamic/mainDynamic.cxx	30 May 2003 20:01:07 -0000
***************
*** 1,6 ****
  // mainDynamic.cxx - high-tech mainline.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2001, 2002 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
--- 1,6 ----
  // mainDynamic.cxx - high-tech mainline.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2001, 2002, 2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
*************** usage ()
*** 120,125 ****
--- 120,129 ----
         << "                         mmap         Memory map given file" << endl
         << "                         latency=r:w  Set read, write latencies [0:0]" << endl
         << "                         latency=rw   Set both latencies [0]" << endl;
+   cout << "--ulog-level=LEVEL    Set the logging level for the current board" << endl;
+   cout << "--ulog-mode=less|match|equal" << endl
+        << "                      Set the logging mode for the current board" << endl;
+   cout << "--ulog-file=*|FILE    Set the log file name" << endl;
    cout << endl
         << " note: most board-specific options can be used in board-neutral position " << endl
         << " where they are interpreted as session-specific or default settings. " << endl;
*************** struct Defs {
*** 462,467 ****
--- 466,474 ----
  	    trace_disassemble (false),
  	    trace_counter (false),
  	    trace_core (false),
+ 	    ulog_level (0),
+ 	    ulog_mode ("less"),
+ 	    ulog_file ("*"),
  	    step_insn_count ("10000")
    {}
    string cpu;
*************** struct Defs {
*** 472,477 ****
--- 479,487 ----
    bool trace_disassemble;
    bool trace_counter;
    bool trace_core;
+   sid::host_int_4 ulog_level;
+   string ulog_mode;
+   string ulog_file;
    string step_insn_count;
  };
    
*************** main(int argc, char* argv[])
*** 514,520 ****
  		    opt_insn_count, opt_load, opt_icache, opt_dcache, 
  		    opt_memory_region, opt_trace_extract, opt_trace_semantics,
  		    opt_trace_disassemble, opt_trace_counter, opt_trace_core,
! 		    opt_final_insn_count, opt_eb, opt_el, opt_gprof };
  		    
    int curr_opt;
  
--- 524,531 ----
  		    opt_insn_count, opt_load, opt_icache, opt_dcache, 
  		    opt_memory_region, opt_trace_extract, opt_trace_semantics,
  		    opt_trace_disassemble, opt_trace_counter, opt_trace_core,
! 		    opt_final_insn_count, opt_eb, opt_el, opt_gprof,
! 		    opt_ulog_level, opt_ulog_mode, opt_ulog_file };
  		    
    int curr_opt;
  
*************** main(int argc, char* argv[])
*** 559,564 ****
--- 570,578 ----
      {"final-insn-count",  no_argument, & curr_opt, opt_final_insn_count },
      {"EB",                no_argument, & curr_opt, opt_eb },
      {"EL",                no_argument, & curr_opt, opt_el },
+     {"ulog-level",        required_argument, &curr_opt, opt_ulog_level },
+     {"ulog-mode",         required_argument, &curr_opt, opt_ulog_mode },
+     {"ulog-file",         required_argument, &curr_opt, opt_ulog_file },
      { 0, 0, NULL, 0 }
   };
    
*************** main(int argc, char* argv[])
*** 632,637 ****
--- 646,654 ----
  			  curr_board->trace_core();
  			if (defaults.enable_warnings)
  			  curr_board->enable_warnings();
+ 			curr_board->set_ulog_level (defaults.ulog_level);
+ 			curr_board->set_ulog_mode (defaults.ulog_mode);
+ 			curr_board->set_ulog_file (defaults.ulog_file);
  			if (defaults.step_insn_count != "10000")
  			  curr_board->set_step_insn_count(defaults.step_insn_count);
  			break;
*************** main(int argc, char* argv[])
*** 815,820 ****
--- 832,871 ----
  		    exit (8);
  		  }
  	      }
+ 	      break;
+ 
+ 	    case opt_ulog_level:
+ 	      if (curr_board)
+ 		curr_board->set_ulog_level (optaddr ("ulog-level"));
+ 	      else
+ 		{
+ 		  defaults.ulog_level = optaddr ("ulog-level");
+ 		  need_sess (sess);
+ 		  sess->set_ulog_level (optaddr ("ulog-level"));
+ 		}
+ 	      break;
+ 
+ 	    case opt_ulog_mode:
+ 	      if (curr_board)
+ 		curr_board->set_ulog_mode (optstring ());
+ 	      else
+ 		{
+ 		  defaults.ulog_mode = optstring ();
+ 		  need_sess (sess);
+ 		  sess->set_ulog_mode (optstring ());
+ 		}
+ 	      break;
+ 
+ 	    case opt_ulog_file:
+ 	      need_sess (sess);
+ 	      sess->add_ulog_file (optstring ());
+ 	      if (curr_board)
+ 		curr_board->set_ulog_file (optstring ());
+ 	      else
+ 		{
+ 		  defaults.ulog_file = optstring ();
+ 		  sess->set_ulog_file (optstring ());
+ 		}
  	      break;
  	    }
  	  break;
Index: sid/component/consoles/sid-io-stdio.txt
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/sid-io-stdio.txt,v
retrieving revision 1.2
diff -c -p -r1.2 sid-io-stdio.txt
*** sid/component/consoles/sid-io-stdio.txt	3 Aug 2001 01:30:17 -0000	1.2
--- sid/component/consoles/sid-io-stdio.txt	30 May 2003 20:23:25 -0000
*************** Functionality:
*** 28,33 ****
--- 28,39 ----
     | output | When the stdout input pin is driven    |
     |        | with a value between 0 and 255, the    |
     |        | value is immediately written to        |
+    |        | output. If the filename attribute is   |
+    |        | set, the output is written to the file |
+    |        | named by the attribute's value. If the |
+    |        | filename attribute is set to "*", then |
+    |        | output is written to stdout. The       |
+    |        | default behavior is to write to        |
     |        | stdout.                                |
     |--------+----------------------------------------|
     |  input | When the poll pin is driven, the stdin |
*************** Component Reference:
*** 106,109 ****
--- 112,119 ----
     |--------------+--------+------+-------+------------||
     |state-snapshot|-       |opaque|-      |state       ||
     |              |        |string|       |save/restore||
+    |--------------+--------+------+-------+------------||
+    |              |        |valid |       |            ||
+    |filename      |-       |file  |-      |output      ||
+    |              |        |name  |       |            ||
     +----------------------------------------------------+
Index: sid/component/consoles/sid-io-stdio.xml
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/sid-io-stdio.xml,v
retrieving revision 1.1
diff -c -p -r1.1 sid-io-stdio.xml
*** sid/component/consoles/sid-io-stdio.xml	3 Aug 2001 01:47:52 -0000	1.1
--- sid/component/consoles/sid-io-stdio.xml	30 May 2003 20:23:25 -0000
***************
*** 13,18 ****
--- 13,19 ----
  
      <!-- attributes -->
      <defattribute name="state-snapshot" legalvalues="opaque string" behaviors="state save/restore" />
+     <defattribute name="filename" legalvalues="valid file name" behaviors="output" />
    </defcomponent>
    <synop>
    This component performs input/output on the standard input/output. 
***************
*** 40,46 ****
  
      <behavior name="output">
      When the <pin>stdout</pin> input pin is driven with a value between 0 and
!     255, the value is immediately written to stdout.
      </behavior>
  
      <behavior name="input">
--- 41,51 ----
  
      <behavior name="output">
      When the <pin>stdout</pin> input pin is driven with a value between 0 and
!     255, the value is immediately written to output. If the
!     <attribute>filename</attribute> attribute
!     is set, the output is written to the file named by the attribute's value. If
!     the <attribute>filename</attribute> attribute is set to "*", then output is
!     written to stdout. The default behavior is to write to stdout.
      </behavior>
  
      <behavior name="input">

Re: [patch] Add Logging Support for SID Components

[Dave Brolley]

I forgot to explain what the new options actually do ......

o To use the logging facility a component inherits from 
fixed_attribute_map_with_logging_component
o The component's methods may then call the method
        log (sid::host_int_4 level, const char *fmt, ...)
o The component's ulog_out_pin will then be driven, once character at a 
time with the log message under the following conditions:
    - If the component's 'ulog-mode' attribute is "less" then the 
message will be output of the 'level' on the call to 'log' is less than 
or equal to the value of the component's 'ulog-level' attribute
    - If the component's 'ulog-mode' attribute is "equal" or "match" 
then the message will be output of the 'level' on the call to 'log' is 
equal to the value of the component's 'ulog-level' attribute

The command line option settings are stored in SessionCfg (specified 
before --board) and BoardCfg (specified after --board) and can be used 
to set these attributes for components using logging. If a sid-io-stdio 
component is connected to the component's ulog_out_pin, then 
--ulog-filename can be used to set its 'filename' attribute.

By default, a component inheriting from 
fixed_attribute_map_with_logging_component is set up for --ulog-level==0 
and --ulog-mode=less. By default a sid-io-stdout component writes to 
stdout. So, the default settings are for logging to stdout for log 
messages with a level of 0.

Dave

Dave Brolley wrote:

> Hi,
>
> The attached patch adds logging support for sid components in 3 stages:
>
> 1) Add a new level to the sid component heirarchy
>
>    o fixed_attribute_map_with_logging_component inherits 
> fixed_attribute_map_component and provides the logging capability
>    o An output pin, ulog_out_pin, is defined here through which the 
> log data is output.
>    o printf-like logging (my immediate need) is provided via the 'void 
> log (level, const char *fmt, ...)' method
>    o Attributes record the logging settings (see options below)
>    o A nice future addition would be a 'log (level)' method which 
> would return some kind of stream object.
>
> 2) Add capability to sid-io-stdio (sid/consoles/stdio.cxx) for writing 
> output to a file.
>
>    o This provides logging to stdout or to a via via the component's 
> ulog_out_pin
>
> 3) Expand the command line interface for sid
>
>    o 3 new options control logging behavior
>        --ulog-level=INTEGER
>        --ulog-mode={less|match|equal}
>        --ulog-file=FILENAME
>    o As with many options, these relate to the previous --board 
> specified or provide settings for all boards if specified before the 
> first --board option.
>    o The board level settings are kept in BoardCfg 
> (sid/main/dynamic/commonCfg.h) by inhereting a new UlogCfg
>    o The global level settings are kept in SessionCfg also by 
> inherting from UlogCfg
>    o Configuration may then use this info to set attributes and 
> connect pins in the desired components to activate the logging feature.
>
> NOTES:
> o I needed to use vsnprintf or vasprintf if available and so I added 
> checks for them in sid/component/configure.in
> o I needed to define _ISOC99_SOURCE for vsnprintf or _GNU_SOURCE for 
> vasprintf and so we now also have to define _BSD_SOURCE and 
> _SVID_SOURCE which are normally defined by default if no _XXXXX_SOURCE 
> is defined.
> o I included the regenerated sid/component/config.h in the patch so 
> you can see the new macros.
>
> I'm requesting review of this patch, especially the placement of the 
> new functionality in the component heirarchy and the configury changes 
> to sid/component.
>
> Thanks,
> Dave

Re: [patch] Add Logging Support for SID Components

[Frank Ch. Eigler]

Hi -

> The attached patch adds logging support for sid components in 3 stages:
> [...]

Looks fine - good use of virtual inheritance.  A few minor things
I might have done differently:

- make a new component type for named file I/O, instead of
  changing sid-io-stdio; after all, stdio is bidirectional,
  meant to be interactive
- think of some other way to call stdout than "*"
- make logging output buffering dependent on an attribute (set as the last
  gasp of configuration) instead of pin-connectedness
- having a C++ flavour API (instead of printf()) for the logging function


- FChE

Re: [patch] Add Logging Support for SID Components

[Ben Elliston]

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

> - think of some other way to call stdout than "*"

How about the more conventional `-'?

Ben

Re: [patch] Add Logging Support for SID Components

[Dave Brolley]

Ben Elliston wrote:

>"Frank Ch. Eigler" <fche@redhat.com> writes:
>
>>- think of some other way to call stdout than "*"
>>
>
>How about the more conventional `-'?
>
That's what I was going for. Just got the wrong character.....

Re: [patch] Add Logging Support for SID Components

[Dave Brolley]

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]

This new patch replaces the previous one and addresses some of the 
concerns below....

Frank Ch. Eigler wrote:

>Hi -
>
>  
>
>>The attached patch adds logging support for sid components in 3 stages:
>>[...]
>>    
>>
>
>Looks fine - good use of virtual inheritance.  A few minor things
>I might have done differently:
>
>- make a new component type for named file I/O, instead of
>  changing sid-io-stdio; after all, stdio is bidirectional,
>  meant to be interactive
>
New component: sid-io-fileio

>- think of some other way to call stdout than "*"
>
Used "-" as originally intended and suggested by bje

>- make logging output buffering dependent on an attribute (set as the last
>  gasp of configuration) instead of pin-connectedness
>
Done

>- having a C++ flavour API (instead of printf()) for the logging function
>
A project for someone's copious spare time.

OK to commit?

Dave

[-- Attachment #2: ulog2.patch.txt --]
[-- Type: text/plain, Size: 29096 bytes --]

? sid/component/consoles/fileio.cxx
? sid/component/consoles/sid-io-fileio.txt
? sid/component/consoles/sid-io-fileio.xml
Index: cgen/sid-cpu.scm
===================================================================
RCS file: /cvs/src/src/cgen/sid-cpu.scm,v
retrieving revision 1.9
diff -c -p -r1.9 sid-cpu.scm
*** cgen/sid-cpu.scm	15 May 2003 07:25:02 -0000	1.9
--- cgen/sid-cpu.scm	9 Jun 2003 12:41:01 -0000
*************** using namespace @cpu@;
*** 595,600 ****
--- 595,603 ----
  		  copyright-red-hat package-red-hat-simulators)
     "\
  
+ #if HAVE_CONFIG_H
+ #include \"config.h\"
+ #endif
  #include \"@cpu@.h\"
  
  using namespace @cpu@; // FIXME: namespace organization still wip
Index: cgen/sid-decode.scm
===================================================================
RCS file: /cvs/src/src/cgen/sid-decode.scm,v
retrieving revision 1.10
diff -c -p -r1.10 sid-decode.scm
*** cgen/sid-decode.scm	15 May 2003 07:25:02 -0000	1.10
--- cgen/sid-decode.scm	9 Jun 2003 12:41:01 -0000
*************** typedef UINT @prefix@_insn_word;
*** 768,773 ****
--- 768,776 ----
  		  copyright-red-hat package-red-hat-simulators)
     "\
  
+ #if HAVE_CONFIG_H
+ #include \"config.h\"
+ #endif
  #include \"@cpu@.h\"
  
  using namespace @cpu@; // FIXME: namespace organization still wip
Index: cgen/sid-model.scm
===================================================================
RCS file: /cvs/src/src/cgen/sid-model.scm,v
retrieving revision 1.5
diff -c -p -r1.5 sid-model.scm
*** cgen/sid-model.scm	15 May 2003 07:25:02 -0000	1.5
--- cgen/sid-model.scm	9 Jun 2003 12:41:01 -0000
*************** const MACH " (gen-sym mach) "_mach =
*** 496,501 ****
--- 496,504 ----
  		  copyright-red-hat package-red-hat-simulators)
     "\
  
+ #if HAVE_CONFIG_H
+ #include \"config.h\"
+ #endif
  #include \"@cpu@.h\"
  
  using namespace @cpu@; // FIXME: namespace organization still wip
Index: sid/component/config.in
===================================================================
RCS file: /cvs/src/src/sid/component/config.in,v
retrieving revision 1.5
diff -c -p -r1.5 config.in
*** sid/component/config.in	11 Nov 2002 22:28:26 -0000	1.5
--- sid/component/config.in	9 Jun 2003 12:41:13 -0000
***************
*** 36,41 ****
--- 36,47 ----
  /* Define if you have the usleep function.  */
  #undef HAVE_USLEEP
  
+ /* Define if you have the vasprintf function.  */
+ #undef HAVE_VASPRINTF
+ 
+ /* Define if you have the vsnprintf function.  */
+ #undef HAVE_VSNPRINTF
+ 
  /* Define if you have the <../ucbinclude/sys/ioctl.h> header file.  */
  #undef HAVE____UCBINCLUDE_SYS_IOCTL_H
  
***************
*** 101,106 ****
--- 107,124 ----
  
  /* Define if curses library is usable */
  #undef HAVE_CURSES_LIBRARY
+ 
+ /* Define if ISO C99 Features in use */
+ #undef _ISOC99_SOURCE
+ 
+ /* Define if GNU extensions in use */
+ #undef _GNU_SOURCE
+ 
+ /* Define if SVID extensions in use */
+ #undef _SVID_SOURCE
+ 
+ /* Define if SVID extensions in use */
+ #undef _BSD_SOURCE
  
  /* Define if ftime(3) is declared via sys/timeb.h */
  #undef HAVE_FTIME_DECL
Index: sid/component/configure.in
===================================================================
RCS file: /cvs/src/src/sid/component/configure.in,v
retrieving revision 1.10
diff -c -p -r1.10 configure.in
*** sid/component/configure.in	6 May 2003 17:30:58 -0000	1.10
--- sid/component/configure.in	9 Jun 2003 12:41:14 -0000
*************** AC_CHECK_LIB(nsl, gethostbyname, socket_
*** 43,50 ****
  		socket_libs="$socket_libs -lresolv"))
  AC_SUBST(socket_libs)
  
! AC_CHECK_FUNCS(ftime times gettimeofday usleep strtoul strtoull select sched_yield)
  
  old_libs="$LIBS"
  LIBS="$LIBS $socket_libs"
  AC_CHECK_FUNCS(inet_aton inet_addr)
--- 43,61 ----
  		socket_libs="$socket_libs -lresolv"))
  AC_SUBST(socket_libs)
  
! AC_CHECK_FUNCS(ftime times gettimeofday usleep strtoul strtoull select sched_yield vsnprintf vasprintf)
  
+ dnl We want vsnprintf if available, otherwise try vasprintf
+ AC_CHECK_FUNC(vsnprintf,
+    [AC_DEFINE(_ISOC99_SOURCE, 1, [Define if ISO C99 Features in use])],
+    [AC_CHECK_FUNC(vasprintf,
+       [AC_DEFINE(_GNU_SOURCE, 1, [Define if GNU extensions in use])])])
+ 
+ dnl To pick up __USE_MISC stuff like inet_aton and putenv
+ AC_DEFINE(_SVID_SOURCE, 1, [Define if SVID extensions in use])
+ dnl To pick up __USE_BSD stuff like usleep
+ AC_DEFINE(_BSD_SOURCE, 1, [Define if SVID extensions in use])
+   
  old_libs="$LIBS"
  LIBS="$LIBS $socket_libs"
  AC_CHECK_FUNCS(inet_aton inet_addr)
Index: sid/component/cgen-cpu/config.in
===================================================================
RCS file: /cvs/src/src/sid/component/cgen-cpu/config.in,v
retrieving revision 1.2
diff -c -p -r1.2 config.in
*** sid/component/cgen-cpu/config.in	8 Jan 2003 04:10:05 -0000	1.2
--- sid/component/cgen-cpu/config.in	9 Jun 2003 12:41:14 -0000
***************
*** 1,5 ****
--- 1,8 ----
  /* config.in.  Generated automatically from configure.in by autoheader.  */
  
+ /* Define if you have the getopt function.  */
+ #undef HAVE_GETOPT
+ 
  /* Define if you have the m library (-lm).  */
  #undef HAVE_LIBM
  
***************
*** 17,20 ****
--- 20,26 ----
  
  /* Define to 1 if we found this declaration otherwise define to 0. */
  #undef HAVE_DECL_BASENAME
+ 
+ /* Define to 1 if we found this declaration otherwise define to 0. */
+ #undef HAVE_DECL_GETOPT
  
Index: sid/component/cgen-cpu/configure.in
===================================================================
RCS file: /cvs/src/src/sid/component/cgen-cpu/configure.in,v
retrieving revision 1.4
diff -c -p -r1.4 configure.in
*** sid/component/cgen-cpu/configure.in	8 Jan 2003 04:10:05 -0000	1.4
--- sid/component/cgen-cpu/configure.in	9 Jun 2003 12:41:15 -0000
*************** AC_EXEEXT
*** 14,19 ****
--- 14,20 ----
  AC_LANG_CPLUSPLUS
  
  AC_CHECK_LIB(m, main)
+ AC_CHECK_FUNCS(getopt)
  
  dnl Check if we need libintl.
  AC_MSG_CHECKING([whether libintl is required for gettext])
*************** AC_CHECK_PROGS(GUILE, guile, false)
*** 59,65 ****
  dnl building docs
  RH_DOCBUILD_CHECK
  
! gcc_AC_CHECK_DECLS([vasprintf asprintf basename], [], [], [
  #include <stdio.h>
  #include <string.h>
  ])
--- 60,66 ----
  dnl building docs
  RH_DOCBUILD_CHECK
  
! gcc_AC_CHECK_DECLS([vasprintf asprintf basename getopt], [], [], [
  #include <stdio.h>
  #include <string.h>
  ])
Index: sid/component/consoles/Makefile.am
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/Makefile.am,v
retrieving revision 1.3
diff -c -p -r1.3 Makefile.am
*** sid/component/consoles/Makefile.am	4 Aug 2001 10:51:59 -0000	1.3
--- sid/component/consoles/Makefile.am	9 Jun 2003 12:41:16 -0000
*************** AUTOMAKE_OPTIONS = foreign
*** 5,15 ****
  pkglib_LTLIBRARIES = libconsoles.la
  
  INCLUDES = -I. -I../../include -I$(srcdir)/../../include
! libconsoles_la_SOURCES = stdio.cxx socketio.cxx components.cxx
  libconsoles_la_LDFLAGS = -module -no-undefined
  libconsoles_la_LIBADD = $(socket_libs)
  
! pkgdata_DATA = sid-io-stdio.txt sid-io-socket.txt hw-visual-tty.tk
  
  DEJAGNUTESTS=stdio.exp consdocs.exp consmonkey.exp stdioval.exp stdioinv.exp socketval.exp socketinv.exp
  check-local:
--- 5,15 ----
  pkglib_LTLIBRARIES = libconsoles.la
  
  INCLUDES = -I. -I../../include -I$(srcdir)/../../include
! libconsoles_la_SOURCES = stdio.cxx fileio.cxx socketio.cxx components.cxx
  libconsoles_la_LDFLAGS = -module -no-undefined
  libconsoles_la_LIBADD = $(socket_libs)
  
! pkgdata_DATA = sid-io-stdio.txt sid-io-fileio.txt sid-io-socket.txt hw-visual-tty.tk
  
  DEJAGNUTESTS=stdio.exp consdocs.exp consmonkey.exp stdioval.exp stdioinv.exp socketval.exp socketinv.exp
  check-local:
Index: sid/component/consoles/components.cxx
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/components.cxx,v
retrieving revision 1.1
diff -c -p -r1.1 components.cxx
*** sid/component/consoles/components.cxx	7 Dec 2000 19:30:50 -0000	1.1
--- sid/component/consoles/components.cxx	9 Jun 2003 12:41:16 -0000
***************
*** 1,7 ****
  // components.cxx - The component_library routines for the various
  // nearby components.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000 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 ----
  // components.cxx - The component_library routines for the various
  // nearby components.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
*************** compConsoleListTypes()
*** 16,21 ****
--- 16,22 ----
  {
    vector<string> types;
    types.push_back("sid-io-stdio");
+   types.push_back("sid-io-fileio");
    types.push_back("sid-io-socket-server");
    types.push_back("sid-io-socket-client");
    return types;
*************** compConsoleCreate(const string& typeName
*** 26,31 ****
--- 27,34 ----
  {
    if (typeName == "sid-io-stdio")
      return new stdioConsole();
+   else if (typeName == "sid-io-fileio")
+     return new fileioConsole();
    else if (typeName == "sid-io-socket-server")
      return new socketio(true);
    else if (typeName == "sid-io-socket-client")
*************** compConsoleDelete(component* c)
*** 41,48 ****
    // One of these dynamic_cast<>s will return 0.  It is safe to delete 0.
    stdioConsole* c1 = dynamic_cast<stdioConsole*>(c);
    if (c1) { delete c1; return; }
!   socketio* c2 = dynamic_cast<socketio*>(c);
    if (c2) { delete c2; return; }
  }
  
  } // end of namespace console_stuff
--- 44,53 ----
    // One of these dynamic_cast<>s will return 0.  It is safe to delete 0.
    stdioConsole* c1 = dynamic_cast<stdioConsole*>(c);
    if (c1) { delete c1; return; }
!   fileioConsole* c2 = dynamic_cast<fileioConsole*>(c);
    if (c2) { delete c2; return; }
+   socketio* c3 = dynamic_cast<socketio*>(c);
+   if (c3) { delete c3; return; }
  }
  
  } // end of namespace console_stuff
Index: sid/component/consoles/components.h
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/components.h,v
retrieving revision 1.3
diff -c -p -r1.3 components.h
*** sid/component/consoles/components.h	3 Aug 2001 06:02:43 -0000	1.3
--- sid/component/consoles/components.h	9 Jun 2003 12:41:16 -0000
***************
*** 1,7 ****
  // file.cxx - Joint header file for nearby component classes.
  // -*- C++ -*-
  
! // Copyright (C) 1999, 2000 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 ----
  // file.cxx - Joint header file for nearby component classes.
  // -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
*************** private:
*** 109,115 ****
--- 109,144 ----
    // save & restore state
    string save_state ( );
    component::status restore_state (const string& state);
+ };
  
+ 
+ // -------------------------------------------------------------------------
+ 
+ class fileioConsole: public virtual component, 
+ 		    protected fixed_attribute_map_component,
+ 		    protected no_bus_component,
+ 		    protected fixed_pin_map_component,
+ 		    protected no_accessor_component,
+ 		    protected no_relation_component
+ {
+ public:
+   fileioConsole();
+  ~fileioConsole () throw();
+ 
+ private:
+   void filename_attribute_set ();
+ 
+   void write(host_int_4 value);
+   void closeOutStream ();
+ 
+   callback_pin<fileioConsole> out_pin;
+ 
+   // save & restore state
+   string save_state ( );
+   component::status restore_state (const string& state);
+ 
+   string out_filename;
+   ostream *out_stream;
  };
  
  
Index: sid/component/consoles/sid-io-stdio.txt
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/sid-io-stdio.txt,v
retrieving revision 1.2
diff -c -p -r1.2 sid-io-stdio.txt
*** sid/component/consoles/sid-io-stdio.txt	3 Aug 2001 01:30:17 -0000	1.2
--- sid/component/consoles/sid-io-stdio.txt	9 Jun 2003 14:08:59 -0000
*************** Environment:
*** 53,58 ****
--- 53,60 ----
  
     Related components
  
+    See sid-io-fileio for writing to a stream.
+ 
     You can connect a stdio console to a serial data transmission component,
     such as a UART. You can also connect it to a ROM monitor/system call
     emulator, such as the sw-gloss-arm/angel which performs I/O across some
Index: sid/component/consoles/sid-io-stdio.xml
===================================================================
RCS file: /cvs/src/src/sid/component/consoles/sid-io-stdio.xml,v
retrieving revision 1.1
diff -c -p -r1.1 sid-io-stdio.xml
*** sid/component/consoles/sid-io-stdio.xml	3 Aug 2001 01:47:52 -0000	1.1
--- sid/component/consoles/sid-io-stdio.xml	9 Jun 2003 14:08:59 -0000
***************
*** 62,67 ****
--- 62,70 ----
      <title>Related components</title>
  
  <p>
+     See sid-io-fileio for writing to a stream.
+ </p>
+ <p>
      You can connect a stdio console to a serial data transmission
      component, such as a UART. You can also connect it to a 
      ROM monitor/system call emulator, such as the <complib>sw-gloss-arm/angel</complib> 
Index: sid/component/memory/generic.cxx
===================================================================
RCS file: /cvs/src/src/sid/component/memory/generic.cxx,v
retrieving revision 1.7
diff -c -p -r1.7 generic.cxx
*** sid/component/memory/generic.cxx	4 Aug 2001 11:34:52 -0000	1.7
--- sid/component/memory/generic.cxx	9 Jun 2003 12:41:16 -0000
***************
*** 1,10 ****
  // generic.cxx - a class of generic memories.  -*- 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.
- 
- #define _POSIX_C_SOURCE 199506L
  
  #include "config.h"
  #include "generic.h"
--- 1,8 ----
  // generic.cxx - a class of generic memories.  -*- C++ -*-
  
! // Copyright (C) 1999-2001,2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
  #include "config.h"
  #include "generic.h"
Index: sid/include/sidattrutil.h
===================================================================
RCS file: /cvs/src/src/sid/include/sidattrutil.h,v
retrieving revision 1.3
diff -c -p -r1.3 sidattrutil.h
*** sid/include/sidattrutil.h	22 Nov 2002 20:35:00 -0000	1.3
--- sid/include/sidattrutil.h	9 Jun 2003 12:41:17 -0000
***************
*** 2,8 ****
  // mappings between application objects and their string
  // representations.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2002 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
--- 2,8 ----
  // mappings between application objects and their string
  // representations.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2002, 2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
***************
*** 10,15 ****
--- 10,16 ----
  #define SIDATTRUTIL_H
  
  #include <sidconfig.h>
+ #include <sidpinutil.h>
  #include "sidtypes.h"
  
  #include <string>
***************
*** 26,35 ****
--- 27,39 ----
  
  
  #include <limits.h>
+ #include <stdio.h>
+ #include <stdarg.h>
  
  // <ext/hash_map>? <hash_map>?  std::?  __gnu_cxx::?  Too much hassle.
  #undef HAVE_HASHING
  
+ #include <iostream>
  #if HAVE_SSTREAM
  #include <sstream>
  #elif HAVE_STRSTREAM_H
*************** make_attribute (const sid::any_int<IntTy
*** 1010,1015 ****
--- 1014,1148 ----
  	    else
  	      return ((*it).second)->parse_attribute (value);
        }
+   };
+ 
+   // A mix-in for classes with user logging.
+   class fixed_attribute_map_with_logging_component
+   : public virtual fixed_attribute_map_component,
+     public virtual fixed_pin_map_component
+   {
+   protected:
+     fixed_attribute_map_with_logging_component () :
+       ulog_level (0),
+       ulog_mode ("less"),
+       ulog_out_pin (),
+       buffer_output (true),
+       buffer_size (4096), // big enough for now
+       saved_messages (),
+       saved_levels ()
+       {
+ 	add_attribute ("buffer-output", &buffer_output, "setting");
+ 	add_attribute ("ulog-level", &ulog_level, "setting");
+ 	add_attribute ("ulog-mode",  &ulog_mode,  "setting");
+ 	add_pin ("ulog-out", & ulog_out_pin);
+ #if HAVE_VSNPRINTF || ! HAVE_VASPRINTF
+ 	buffer = new char[buffer_size];
+ #endif
+       }
+     ~fixed_attribute_map_with_logging_component () /* throw() */
+       {
+ 	// Output any saved messages.
+ 	output_saved_messages ();
+ #if HAVE_VSNPRINTF || ! HAVE_VASPRINTF
+ 	delete [] buffer;
+ #endif
+       }
+ 
+     virtual void log (sid::host_int_4 level, const char *fmt, ...)
+       {
+ 	if (! buffer_output)
+ 	  {
+ 	    // Output any saved messages first
+ 	    output_saved_messages ();
+ 
+ 	    // Check the logging level and mode.
+ 	    if (! check_level (level))
+ 	      return;
+ 	  }
+ 
+ 	// Write the message into a buffer.
+ 	int length;
+ 	for (;;)
+ 	  {
+ 	    va_list ap;
+ 	    va_start (ap, fmt);
+ #if HAVE_VSNPRINTF
+ 	    length = vsnprintf (buffer, buffer_size, fmt, ap);
+ 	    va_end (ap);
+ 	    if (length < buffer_size)
+ 	      break;
+ 	    delete [] buffer;
+ 	    buffer_size = length + 256;
+ 	    buffer = new char[buffer_size];
+ #elif HAVE_VASPRINTF
+ 	    length = vasprintf (&buffer, fmt, ap);
+ 	    va_end (ap);
+ 	    break;
+ #else
+ 	    length = STDCTYPE(vsprintf) (buffer, fmt, ap);
+ 	    va_end (ap);
+ 	    if (length >= buffer_size)
+ 	      std::cerr << "Error: ulog buffer overflow!!!" << std::endl;
+ 	    break;
+ #endif
+ 	  }
+ 
+ 	// If the output pin is not connected yet, Save the message for
+ 	// later. This happens when the log message is issued from the
+ 	// component's constructor.
+ 	if (buffer_output)
+ 	  {
+ 	    saved_messages.push_back (std::string (buffer));
+ 	    saved_levels.push_back (level);
+ 	  }
+ 	else
+ 	  {
+ 	    // Otherwise, output the new message.
+ 	    for (int i = 0; i < length; ++i)
+ 	      ulog_out_pin.drive (buffer[i]);
+ 	  }
+ 
+ #if ! HAVE_VSNPRINTF && HAVE_VASPRINTF
+ 	free (buffer);
+ #endif
+       }
+ 
+ private:
+     bool check_level (sid::host_int_4 level)
+       {
+ 	if (level > ulog_level)
+ 	  return false;
+ 
+ 	if (level != ulog_level
+ 	    && (ulog_mode == "match" || ulog_mode == "equal"))
+ 	  return false;
+ 
+ 	return true;
+       }
+ 
+     void output_saved_messages ()
+       {
+ 	while (saved_messages.size () > 0)
+ 	  {
+ 	    if (check_level (saved_levels[0]))
+ 	      {
+ 		std::string s = saved_messages[0];
+ 		for (int i = 0; i < s.size (); ++i)
+ 		  ulog_out_pin.drive (s[i]);
+ 	      }
+ 	    saved_messages.erase (saved_messages.begin ());
+ 	    saved_levels.erase (saved_levels.begin ());
+ 	  }
+       }
+ 
+     sid::host_int_4 ulog_level;
+     std::string ulog_mode;
+     sidutil::output_pin ulog_out_pin;
+     bool buffer_output;
+     char *buffer;
+     long buffer_size;
+     std::vector<std::string> saved_messages;
+     std::vector<sid::host_int_4> saved_levels;
    };
  }
  
Index: sid/main/dynamic/commonCfg.cxx
===================================================================
RCS file: /cvs/src/src/sid/main/dynamic/commonCfg.cxx,v
retrieving revision 1.3
diff -c -p -r1.3 commonCfg.cxx
*** sid/main/dynamic/commonCfg.cxx	16 Apr 2003 22:01:07 -0000	1.3
--- sid/main/dynamic/commonCfg.cxx	9 Jun 2003 12:41:17 -0000
*************** SessionCfg::SessionCfg (const string nam
*** 545,550 ****
--- 545,573 ----
    conn_pin (main_obj, "stopping", shutdown_seq, "input");
    yield_net->add_output (0, host_sched, "yield");
    init_seq->add_output (0, reset_net, "input");
+ 
+   AtomicCfg *ulog = new AtomicCfg ("ulog-*",
+ 				   "libconsoles.la", 
+ 				   "console_component_library", 
+ 				   "sid-io-stdio");
+   ulog_map["*"] = ulog;
+   add_child (ulog);
+ }
+ 
+ void
+ SessionCfg::add_ulog_file (const string name)
+ {
+   if (ulog_map.find (name) != ulog_map.end ())
+     return; // already there
+ 
+   // There is no existing logger for this file, so add a new one.
+   AtomicCfg *ulog = new AtomicCfg ("ulog-" + name, 
+ 				   "libconsoles.la", 
+ 				   "console_component_library", 
+ 				   "sid-io-stdio");
+   set (ulog, "filename", name);
+   ulog_map[name] = ulog;
+   add_child (ulog);
  }
  
  void SessionCfg::use_no_stdio ()
*************** BoardCfg::BoardCfg (const string name, 
*** 830,835 ****
--- 853,859 ----
  		    bool with_cpu_main_mem_connect) :
    ComponentCfg (name),
    AggregateCfg (name),
+   UlogCfg (),
    cache_flush_net (NULL),
    z_packet (with_z_packet),
    sess (s),
*************** void BoardCfg::add_icache (const string 
*** 1078,1084 ****
  void BoardCfg::add_memory (const Mapping &m)
  {
    if (main_mapper)
!     main_mapper->map (m);
  }
  
  
--- 1102,1111 ----
  void BoardCfg::add_memory (const Mapping &m)
  {
    if (main_mapper)
!     {
!       Mapping map(m);
!       main_mapper->map (map.base(0));
!     }
  }
  
  
Index: sid/main/dynamic/commonCfg.h
===================================================================
RCS file: /cvs/src/src/sid/main/dynamic/commonCfg.h,v
retrieving revision 1.1
diff -c -p -r1.1 commonCfg.h
*** sid/main/dynamic/commonCfg.h	9 Jan 2003 04:22:50 -0000	1.1
--- sid/main/dynamic/commonCfg.h	9 Jun 2003 12:41:17 -0000
*************** class GlueSeqCfg : 
*** 166,175 ****
    int n;
  };
  
  // you should really only make one of these, with an empty name,
  // unless you want some crazy multi-session support.
  struct SessionCfg :
!   virtual public AggregateCfg
  {
    SessionCfg (const string name);
    virtual ~SessionCfg ();  
--- 166,189 ----
    int n;
  };
  
+ // Configs which support logging should inherit this
+ struct UlogCfg
+ {
+   UlogCfg (sid::host_int_4 l = 0, const string m = "less", const string f = "-")
+     : ulog_level (l), ulog_mode (m), ulog_file (f)
+   {}
+   virtual void set_ulog_level (sid::host_int_4 l) { ulog_level = l; }
+   virtual void set_ulog_mode (const string m) { ulog_mode = m; }
+   virtual void set_ulog_file (const string f) { ulog_file = f; }
+   sid::host_int_4 ulog_level;
+   string ulog_mode;
+   string ulog_file;
+ };
+ 
  // you should really only make one of these, with an empty name,
  // unless you want some crazy multi-session support.
  struct SessionCfg :
!   virtual public AggregateCfg, public UlogCfg
  {
    SessionCfg (const string name);
    virtual ~SessionCfg ();  
*************** struct SessionCfg :
*** 194,199 ****
--- 208,215 ----
    AtomicCfg *tcl_bridge;
    bool verbose;
    bool use_stdio;
+   void add_ulog_file (const string filename);
+   map<const string, AtomicCfg *> ulog_map;
  };
  
  class CpuCfg :
*************** class GdbCfg :
*** 278,284 ****
  };
  
  class BoardCfg :
!   virtual public AggregateCfg
  {
  public:
    BoardCfg (const string name,
--- 294,300 ----
  };
  
  class BoardCfg :
! virtual public AggregateCfg, public UlogCfg
  {
  public:
    BoardCfg (const string name,
*************** public:
*** 307,312 ****
--- 323,329 ----
    virtual void trace_disassemble ();
    virtual void trace_core ();
    virtual void write_config (Writer &w);
+ 
    virtual ~BoardCfg ();
  
    GlueSeqCfg *cache_flush_net;
Index: sid/main/dynamic/mainDynamic.cxx
===================================================================
RCS file: /cvs/src/src/sid/main/dynamic/mainDynamic.cxx,v
retrieving revision 1.3
diff -c -p -r1.3 mainDynamic.cxx
*** sid/main/dynamic/mainDynamic.cxx	9 Jan 2003 04:22:50 -0000	1.3
--- sid/main/dynamic/mainDynamic.cxx	9 Jun 2003 12:41:17 -0000
***************
*** 1,6 ****
  // mainDynamic.cxx - high-tech mainline.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2001, 2002 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
--- 1,6 ----
  // mainDynamic.cxx - high-tech mainline.  -*- C++ -*-
  
! // Copyright (C) 1999, 2000, 2001, 2002, 2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
*************** usage ()
*** 120,125 ****
--- 120,129 ----
         << "                         mmap         Memory map given file" << endl
         << "                         latency=r:w  Set read, write latencies [0:0]" << endl
         << "                         latency=rw   Set both latencies [0]" << endl;
+   cout << "--ulog-level=LEVEL    Set the logging level for the current board" << endl;
+   cout << "--ulog-mode=less|match|equal" << endl
+        << "                      Set the logging mode for the current board" << endl;
+   cout << "--ulog-file=*|FILE    Set the log file name" << endl;
    cout << endl
         << " note: most board-specific options can be used in board-neutral position " << endl
         << " where they are interpreted as session-specific or default settings. " << endl;
*************** void try_add_memory (const string memspe
*** 381,390 ****
         i != bases.end(); ++i)
      {
        Mapping m = Mapping()
! 	.slave(mem).bus(port).base(0)
  	.low(*i).high((*i) + size - 1);
        if (map)
! 	map->map (m);
        else if (board)
  	board->add_memory (m);
        else
--- 385,394 ----
         i != bases.end(); ++i)
      {
        Mapping m = Mapping()
! 	.slave(mem).bus(port)
  	.low(*i).high((*i) + size - 1);
        if (map)
! 	map->map (m.base(0));
        else if (board)
  	board->add_memory (m);
        else
*************** struct Defs {
*** 462,467 ****
--- 466,474 ----
  	    trace_disassemble (false),
  	    trace_counter (false),
  	    trace_core (false),
+ 	    ulog_level (0),
+ 	    ulog_mode ("less"),
+ 	    ulog_file ("-"),
  	    step_insn_count ("10000")
    {}
    string cpu;
*************** struct Defs {
*** 472,477 ****
--- 479,487 ----
    bool trace_disassemble;
    bool trace_counter;
    bool trace_core;
+   sid::host_int_4 ulog_level;
+   string ulog_mode;
+   string ulog_file;
    string step_insn_count;
  };
    
*************** main(int argc, char* argv[])
*** 514,520 ****
  		    opt_insn_count, opt_load, opt_icache, opt_dcache, 
  		    opt_memory_region, opt_trace_extract, opt_trace_semantics,
  		    opt_trace_disassemble, opt_trace_counter, opt_trace_core,
! 		    opt_final_insn_count, opt_eb, opt_el, opt_gprof };
  		    
    int curr_opt;
  
--- 524,531 ----
  		    opt_insn_count, opt_load, opt_icache, opt_dcache, 
  		    opt_memory_region, opt_trace_extract, opt_trace_semantics,
  		    opt_trace_disassemble, opt_trace_counter, opt_trace_core,
! 		    opt_final_insn_count, opt_eb, opt_el, opt_gprof,
! 		    opt_ulog_level, opt_ulog_mode, opt_ulog_file };
  		    
    int curr_opt;
  
*************** main(int argc, char* argv[])
*** 559,564 ****
--- 570,578 ----
      {"final-insn-count",  no_argument, & curr_opt, opt_final_insn_count },
      {"EB",                no_argument, & curr_opt, opt_eb },
      {"EL",                no_argument, & curr_opt, opt_el },
+     {"ulog-level",        required_argument, &curr_opt, opt_ulog_level },
+     {"ulog-mode",         required_argument, &curr_opt, opt_ulog_mode },
+     {"ulog-file",         required_argument, &curr_opt, opt_ulog_file },
      { 0, 0, NULL, 0 }
   };
    
*************** main(int argc, char* argv[])
*** 632,637 ****
--- 646,654 ----
  			  curr_board->trace_core();
  			if (defaults.enable_warnings)
  			  curr_board->enable_warnings();
+ 			curr_board->set_ulog_level (defaults.ulog_level);
+ 			curr_board->set_ulog_mode (defaults.ulog_mode);
+ 			curr_board->set_ulog_file (defaults.ulog_file);
  			if (defaults.step_insn_count != "10000")
  			  curr_board->set_step_insn_count(defaults.step_insn_count);
  			break;
*************** main(int argc, char* argv[])
*** 815,820 ****
--- 832,871 ----
  		    exit (8);
  		  }
  	      }
+ 	      break;
+ 
+ 	    case opt_ulog_level:
+ 	      if (curr_board)
+ 		curr_board->set_ulog_level (optaddr ("ulog-level"));
+ 	      else
+ 		{
+ 		  defaults.ulog_level = optaddr ("ulog-level");
+ 		  need_sess (sess);
+ 		  sess->set_ulog_level (optaddr ("ulog-level"));
+ 		}
+ 	      break;
+ 
+ 	    case opt_ulog_mode:
+ 	      if (curr_board)
+ 		curr_board->set_ulog_mode (optstring ());
+ 	      else
+ 		{
+ 		  defaults.ulog_mode = optstring ();
+ 		  need_sess (sess);
+ 		  sess->set_ulog_mode (optstring ());
+ 		}
+ 	      break;
+ 
+ 	    case opt_ulog_file:
+ 	      need_sess (sess);
+ 	      sess->add_ulog_file (optstring ());
+ 	      if (curr_board)
+ 		curr_board->set_ulog_file (optstring ());
+ 	      else
+ 		{
+ 		  defaults.ulog_file = optstring ();
+ 		  sess->set_ulog_file (optstring ());
+ 		}
  	      break;
  	    }
  	  break;

[-- Attachment #3: fileio.cxx --]
[-- Type: text/plain, Size: 1567 bytes --]

// fileio.cxx - A simple output console that uses standard I/O for
// enunciation.  -*- C++ -*-

// Copyright (C) 2003 Red Hat.
// This file is part of SID and is licensed under the GPL.
// See the file COPYING.SID for conditions for redistribution.

#include <fstream>
#include "config.h"
#include "components.h"

namespace console_stuff
{

fileioConsole::fileioConsole()
  :out_pin(this, & fileioConsole::write),
   out_filename ("-"),
   out_stream (&cout)
{
  add_pin("write", &out_pin);
  add_attribute_notify ("filename", & out_filename, this, & fileioConsole::filename_attribute_set,
			"output file setting");
  add_attribute_virtual ("state-snapshot", this,
			 & fileioConsole::save_state,
			 & fileioConsole::restore_state);
}

fileioConsole::~fileioConsole () throw ()
{
  closeOutStream ();
}

void
fileioConsole::closeOutStream ()
{
  if (out_stream && out_stream != &cout)
    delete out_stream;
}

void
fileioConsole::filename_attribute_set ()
{
  // Associate the out_stream with the new filename
  closeOutStream ();
  if (out_filename == "-")
    out_stream = &cout;
  else
    out_stream = new std::ofstream (out_filename.c_str ());
}

void
fileioConsole::write(host_int_4 value)
{
  if (out_stream)
    (*out_stream) << static_cast<char> (value) << flush;
}

string 
fileioConsole::save_state ( )
{
  return string("fileioConsole");
}

component::status
fileioConsole::restore_state(const string& state)
{
  if ( state == "fileioConsole")
    return component::ok;
  else
    return component::bad_value;
}


} // end of namespace console_stuff

[-- Attachment #4: sid-io-fileio.txt --]
[-- Type: text/plain, Size: 3084 bytes --]

          sid-io-fileio (libconsoles.la :: console_component_library)

Synopsis:

   This component performs input/output on the standard input/output.

     ----------------------------------------------------------------------

Functionality:

  Modelling:

   This component acts as an interface between the host system's file system
   and pins in the simulated system.

   +-------------------------------------------------+
   |                    Behaviors                    |
   |-------------------------------------------------|
   | output | When the write input pin is driven     |
   |        | with a value between 0 and 255, the    |
   |        | value is immediately written to the    |
   |        | stream associated with this component. |
   |        | Setting the filename attribute         |
   |        | associates a file with the output      |
   |        | stream. Setting this attribute with    |
   |        | the value "-" associates the output    |
   |        | stream with cout. By default, the      |
   |        | output stream is associated with cout. |
   +-------------------------------------------------+

   +-------------------------------------------------+
   |                 SID Conventions                 |
   |-------------------------------------------------|
   |   functional | supported | -                    |
   |--------------+-----------+----------------------|
   | save/restore | supported | null state           |
   |              |           | save/restore.        |
   +-------------------------------------------------+

     ----------------------------------------------------------------------

Environment:

   Related components

   See sid-io-stdio for input and output from stdin/stdout.

     ----------------------------------------------------------------------

Component Reference:

  Component: sid-io-fileio

   +-------------------------------------------------+
   |                      pins                       |
   |-------------------------------------------------|
   | name  | direction |   legalvalues   | behaviors |
   |-------+-----------+-----------------+-----------|
   | write | in        | any character   | output    |
   |       |           | code            |           |
   +-------------------------------------------------+

   +----------------------------------------------------+
   |                     attributes                     |
   |----------------------------------------------------|
   |     name     |category|legal |default| behaviors  ||
   |              |        |values| value |            ||
   |--------------+--------+------+-------+------------||
   |              |        |valid |       |            ||
   |filename      |-       |file  |-      |            ||
   |              |        |name  |       |            ||
   |--------------+--------+------+-------+------------||
   |state-snapshot|-       |opaque|-      |state       ||
   |              |        |string|       |save/restore||
   +----------------------------------------------------+

[-- Attachment #5: sid-io-fileio.xml --]
[-- Type: text/xml, Size: 1599 bytes --]

<?xml version="1.0" ?>
<!DOCTYPE defcomplib SYSTEM "http://sources.redhat.com/sid/component.dtd">

<defcomplib lib="libconsoles.la" dlsym="console_component_library">
  <defcomponent name="sid-io-fileio" type="concrete">


    <!-- pins -->
    <defpin name="write" direction="in" legalvalues="any character code" behaviors="output" />


    <!-- attributes -->
    <defattribute name="filename" legalvalues="valid file name" />
    <defattribute name="state-snapshot" legalvalues="opaque string" behaviors="state save/restore" />
  </defcomponent>
  <synop>
  This component performs input/output on the standard input/output. 
  </synop>
  <func>
    <modelling>

<p>
    This component acts as an interface between the host system's
    file system and pins in the simulated system.
</p>

    </modelling>

    <behavior name="output">
    When the <pin>write</pin> input pin is driven with a value between 0 and
    255, the value is immediately written to the stream associated with this component.

    Setting the <attribute>filename</attribute> attribute associates a file with the
    output stream. Setting this attribute with the value "-" associates the output
    stream with cout.

    By default, the output stream is associated with cout.
    </behavior>

    <convention name="functional" supported="true"/>
    <convention name="save/restore" supported="true">
      <p>null state save/restore.</p>
    </convention>
  </func>
  
  

  <env>
    <title>Related components</title>

<p>
    See sid-io-stdio for input and output from stdin/stdout.
</p>    

  </env>
</defcomplib>

Re: [patch] Add Logging Support for SID Components

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

Hi -

> This new patch replaces the previous one and addresses some of the 
> concerns below....

Addressing my suggestions was not intended as a prerequisite for
committing your previous version.

The new version seems to have at least one actual problem though:
the way that the buffer array is managed.  Its allocation/deallocation
strategy should be consistent (probably "new char[]" and "delete[] char")
regardless of the availability of the various *sprintf variants.  It should
probably not even be in #if/#endif markers.  The new code appears to be
able to hit free() even though malloc() was never called.

- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch] Add Logging Support for SID Components

[Dave Brolley]

Frank Ch. Eigler wrote:

>The new version seems to have at least one actual problem though:
>the way that the buffer array is managed.  Its allocation/deallocation
>strategy should be consistent (probably "new char[]" and "delete[] char")
>regardless of the availability of the various *sprintf variants.  It should
>probably not even be in #if/#endif markers.  The new code appears to be
>able to hit free() even though malloc() was never called.
>  
>
The piece of information that you're probably missing is that vasprintf 
automatically allocates a buffer of the proper size each time it is 
called and it uses malloc. Thus free () is only called when using 
vasprintf (! HAVE_VSNPRINTF && HAVE_VASPRINTF). Otherwise 
(HAVE_VSNPRINTF || ! HAVE_VAPRINTF) a persistent buffer is used  which 
can be grown if necessary only when vsnprintf is used. I chose to use 
new/delete here since I had a choice.

I have reviewed the code again and believe that it is correct. I can get 
the call to free() out of #if/#endif if I allocate a buffer everytime 
for all alternatives using malloc, but conditional compilation would 
still be necessary for the allocations/growth.

Let me know if you feel these changes are necessary.

Thanks,
Dave