I talked with coly about this for a while on IRC.  The patch below looks
solid from a design point of view.  However, there are some fundamenal
things that are complicated to handle, like backspacing to the previous
line, resizing screen size, lines longer than screen width, and a host
of other wierd corner cases. This patch doesn't handle those types of
things.  I'm preferring that we get editline to be able to handle
event-driven input, since editline handles all the strange cases and I
don't want anyone to have to go through the sufferring of writing that
code again.  coly will be working on integrating editline (and, if
anyone knows of a version of editline that already handles this, a
pointer would be great).

The comments I have on this are stylistic and minor:

    * It looks like you used vi with tab stops set to 4.  This is
      generally a bad idea as it screws up viewing for most other
      people.  All indents should be 4 for this code and all tab stops
      should be 8 for all code.
    * You should generally maintain the style of the code that is
      already there.  For instance, when I code in the Linux kernel, I
      use 8 character indent (though I normally use 4) and I use spaces
      in  things like "for (i = 0; i < len; i++) ....".  I do that even
      though I don't like those things very much.  The only thing I
      really see from the patch below is I use spaces after if, while,
      switch and before '{'.  It would be nice to have one coding style,
      but history has not given us one :(.
    * Large arrays of things (the buffers array in this case) is
      generally frowned upon.  It's not a big deal here, but it's
      something to watch out for.  On a 64-bit platform, this would take
      up 32K of RAM.  Plus, if you wanted to add dynamic resizing of the
      history buffer in the future, it would be hard.  A linked list
      might be a better choice.  In some ways this is due to my telecom
      background.  In telecom, systems have to run for a very long time
      (years) and fragmentation is a big deal.  Large allocations tend
      to cause fragmentation.

Thanks for this.

-Corey

coly wrote:

>
> hi, the attachment is a patch for ipmish history commands supporting.
> By this patch, user can re-use the history commands by using arrow
> key(up and down). Hope this will help, for really I want this feature
> for days.
>
> The previous patch is mistaken, the attachment for this email is
> right. really sorry for that.
>
> Coly
>
>
>------------------------------------------------------------------------
>
>Index: ipmish.c
>===================================================================
>RCS file: /cvsroot/openipmi/OpenIPMI/cmdlang/ipmish.c,v
>retrieving revision 1.26
>diff -u -r1.26 ipmish.c
>--- ipmish.c   10 Jan 2006 22:20:53 -0000      1.26
>+++ ipmish.c   24 Jan 2006 06:03:30 -0000
>@@ -70,6 +70,9 @@
> # endif
> #endif
> 
>+/* MAX_CMD_HISTORY_NR should large more than 2 */
>+#define MAX_CMD_HISTORY_NR    50
>+#define MAX_LINE_BUFFER_SIZE  4096
> extern os_handler_t ipmi_debug_os_handlers;
> selector_t *debug_sel;
> 
>@@ -79,12 +82,36 @@
> static int evcount = 0;
> static int handling_input = 0;
> static char *line_buffer;
>-static int  line_buffer_max = 0;
>-static int  line_buffer_pos = 0;
>+static int line_buffer_max = 0;
>+static int line_buffer_pos = 0;
> static int cmd_redisp = 1;
> 
> static void user_input_ready(int fd, void *data, os_hnd_fd_id_t *id);
> 
>+typedef struct history_commands_s{
>+      char *buffers[MAX_CMD_HISTORY_NR];
>+      int buffers_len[MAX_CMD_HISTORY_NR];
>+      int oldest_idx;
>+      int newest_idx;
>+      int current_idx;
>+      int total_nr;
>+}history_commands_t;
>+
>+static history_commands_t history_commands={
>+      .buffers={NULL,},
>+      .buffers_len={-1,},
>+      .oldest_idx=-1,
>+      .newest_idx=-1,
>+      .current_idx=-1,
>+      .total_nr=0,
>+};
>+
>+typedef enum {
>+      LINE_BUFFER,
>+      HISTORY_COMMAND,
>+}input_mode_t;
>+
>+
> static void
> redraw_cmdline(int force)
> {
>@@ -371,10 +398,19 @@
> }
> #endif /* HAVE_UCDSNMP */
> 
>+#define CONTROL(char)  ((char) - '@')
>+typedef enum {
>+      INPUT_NORMAL,
>+      INPUT_PREESC,
>+      INPUT_ESC,
>+}esc_state_t;
>+
> typedef struct out_data_s
> {
>     FILE *stream;
>     int  indent;
>+    esc_state_t  esc_state;
>+    input_mode_t input_mode;    
> } out_data_t;
> 
> static int columns = 80;
>@@ -483,6 +519,8 @@
> {
>     .stream = NULL,
>     .indent = 0,
>+    .esc_state = INPUT_NORMAL,
>+    .input_mode = LINE_BUFFER,
> };
> static char cmdlang_objstr[IPMI_MAX_NAME_LEN];
> static ipmi_cmdlang_t cmdlang =
>@@ -604,6 +642,177 @@
>     redraw_cmdline(0);
> }
> 
>+static int
>+record_history_command(char * line_buffer)
>+{
>+    int buffer_len,i;
>+    int newest_idx, oldest_idx;
>+    int rv=-1;
>+
>+    if(line_buffer[0] == '\0')
>+      goto out;
>+    for(i=0; isspace(line_buffer[i]); i++)
>+          ;
>+    if(line_buffer[i] == '#')
>+      goto out;
>+    buffer_len= strlen(line_buffer) - i ;
>+
>+    newest_idx=history_commands.newest_idx;
>+    oldest_idx=history_commands.oldest_idx;
>+
>+    if((newest_idx != -1) &&
>+      (!strncmp(&line_buffer[i], history_commands.buffers[newest_idx],
>+                      MAX_LINE_BUFFER_SIZE))
>+      )
>+      goto out;
>+
>+    char *newline = ipmi_mem_alloc(buffer_len + 1);
>+    if(newline == NULL)
>+      goto out;
>+
>+    memcpy(newline, &line_buffer[i], buffer_len + 1);
>+
>+    if(newest_idx == -1){
>+      newest_idx=oldest_idx=0;
>+    }else{
>+      newest_idx= (newest_idx + 1)%MAX_CMD_HISTORY_NR;
>+      if(newest_idx == oldest_idx)
>+              oldest_idx= (oldest_idx + 1)%MAX_CMD_HISTORY_NR;
>+    }
>+    if(history_commands.buffers[newest_idx]){
>+      ipmi_mem_free(history_commands.buffers[newest_idx]);
>+        history_commands.buffers[newest_idx]=NULL;
>+    }
>+    history_commands.buffers[newest_idx]=newline;
>+    history_commands.buffers_len[newest_idx]=buffer_len;
>+    history_commands.newest_idx=newest_idx;
>+    history_commands.oldest_idx=oldest_idx;
>+
>+    rv=0;
>+out:    
>+    return rv;
>+}
>+
>+static void
>+erase_chars(FILE *fp, int nr)
>+{
>+      while(nr>0){
>+              fputs("\b \b", fp);
>+              nr -- ;
>+      }
>+}
>+
>+static void
>+fput_chars(FILE *fp, const char * buffer, int nr)
>+{
>+      int i;
>+      for(i=0; i<nr; i++)
>+              fputc(buffer[i], fp);
>+}
>+
>+static void
>+previous_char(FILE *fp)
>+{
>+      if(line_buffer_pos>0){
>+              line_buffer_pos --;
>+              fputs("\b \b", fp);
>+      }
>+}
>+
>+static void
>+previous_history_commands(out_data_t *out_data)
>+{
>+      if(history_commands.newest_idx == -1)   
>+              return;
>+
>+      switch(out_data->input_mode){
>+      case    LINE_BUFFER:
>+                      /* line_buffer[line_buffer_pos] has no input */
>+                      if(line_buffer_pos > 0)
>+                              erase_chars(out_data->stream, line_buffer_pos );
>+                      fflush(out_data->stream);
>+                      
>history_commands.current_idx=history_commands.newest_idx;
>+                      fput_chars(out_data->stream, 
>+                                      
>history_commands.buffers[history_commands.current_idx], 
>+                                      
>history_commands.buffers_len[history_commands.current_idx]);
>+                      out_data->input_mode=HISTORY_COMMAND;
>+                      break;
>+      case    HISTORY_COMMAND:
>+                      if(history_commands.current_idx == 
>history_commands.oldest_idx)
>+                              goto out;
>+                      erase_chars(out_data->stream, 
>history_commands.buffers_len[history_commands.current_idx] );
>+                      fflush(out_data->stream);
>+                      history_commands.current_idx = 
>(history_commands.current_idx + MAX_CMD_HISTORY_NR - 1)%MAX_CMD_HISTORY_NR;
>+                      fput_chars(out_data->stream, 
>+                                      
>history_commands.buffers[history_commands.current_idx], 
>+                                      
>history_commands.buffers_len[history_commands.current_idx]);
>+                      break;
>+      default:
>+                      printf("unknow input mode: %2x\n", 
>out_data->input_mode);
>+                      out_data->input_mode=LINE_BUFFER;
>+                      printf("reset input mode to %2x\n", 
>out_data->input_mode);
>+                      break;  
>+      }
>+out:
>+      return;
>+}
>+
>+static void
>+next_history_commands(out_data_t *out_data)
>+{
>+      switch(out_data->input_mode){
>+      case    LINE_BUFFER:
>+              break;
>+      case    HISTORY_COMMAND:
>+              erase_chars(out_data->stream, 
>history_commands.buffers_len[history_commands.current_idx]);
>+              if(history_commands.current_idx == history_commands.newest_idx){
>+                      history_commands.current_idx=-1;
>+                      fput_chars(out_data->stream, line_buffer, 
>line_buffer_pos);
>+                      out_data->input_mode=LINE_BUFFER;
>+              }else{
>+                      history_commands.current_idx = 
>(history_commands.current_idx + 1)%MAX_CMD_HISTORY_NR;
>+                      fput_chars(out_data->stream, 
>+                                      
>history_commands.buffers[history_commands.current_idx], 
>+                                      
>history_commands.buffers_len[history_commands.current_idx]);
>+              }
>+              break;
>+      default:
>+              fprintf(stderr, "unknow input mode: %2x\n", 
>out_data->input_mode);
>+              out_data->input_mode=LINE_BUFFER;
>+              fprintf(stderr, "reset input mode to %2x\n", 
>out_data->input_mode);
>+              break;
>+      }
>+}
>+static int
>+history_to_buffer()
>+{
>+      char * newline=NULL;
>+      int rv=-1;
>+
>+      
>newline=ipmi_mem_alloc(history_commands.buffers_len[history_commands.current_idx]
> + 1);
>+      if(!newline){
>+              fprintf(stderr, "allocate newline failed.\n");
>+              goto out;
>+      }
>+      if(line_buffer) 
>+      {
>+              ipmi_mem_free(line_buffer);
>+              line_buffer=NULL;
>+              line_buffer_pos=0;
>+      }
>+      line_buffer=newline;
>+      
>line_buffer_pos=history_commands.buffers_len[history_commands.current_idx] ;
>+      
>line_buffer_max=history_commands.buffers_len[history_commands.current_idx] ;
>+      memcpy(line_buffer, 
>+              history_commands.buffers[history_commands.current_idx], 
>+              history_commands.buffers_len[history_commands.current_idx]);
>+      history_commands.current_idx=-1;
>+      rv=0;
>+out:
>+      return rv;
>+}
>+
>+
> static void
> user_input_ready(int fd, void *data, os_hnd_fd_id_t *id)
> {
>@@ -620,71 +829,157 @@
>       return;
>     }
> 
>-    switch(rc) {
>-    case 0x04: /* ^d */
>-      if (line_buffer_pos == 0) {
>-          done = 1;
>-          evcount = 1; /* Force a newline */
>-      }
>-      break;
>-
>-    case 12: /* ^l */
>-      fputc('\n', out_data->stream);
>-      redraw_cmdline(1);
>-      break;
>-
>-    case '\r': case '\n':
>-      fputc(rc, out_data->stream);
>-      if (line_buffer) {
>-          line_buffer[line_buffer_pos] = '\0';
>-          for (i=0; isspace(line_buffer[i]); i++)
>-              ;
>-          /* Ignore blank lines. */
>-          if (line_buffer[i] != '\0') {
>-              /* Turn off input processing. */
>-              disable_term_fd(info);
>-
>-              cmdlang.err = 0;
>-              cmdlang.errstr = NULL;
>-              cmdlang.errstr_dynalloc = 0;
>-              cmdlang.location = NULL;
>-              handling_input = 0;
>-              line_buffer_pos = 0;
>-              ipmi_cmdlang_handle(&cmdlang, line_buffer);
>-          } else {
>-              fputs("> ", out_data->stream);
>-          }
>-      } else {
>-          fputs("> ", out_data->stream);
>+    if(out_data->esc_state == INPUT_PREESC){
>+      switch(rc){
>+      case '[':
>+              out_data->esc_state = INPUT_ESC;
>+              break;
>+      default:
>+              out_data->esc_state = INPUT_NORMAL;
>+              break;
>       }
>-      break;
>-
>-    case 0x7f: /* delete */
>-    case '\b': /* backspace */
>-      if (line_buffer_pos > 0) {
>-          line_buffer_pos--;
>-          fputs("\b \b", out_data->stream);
>+    }else if(out_data->esc_state == INPUT_ESC){
>+      switch(rc){
>+      /* previous line */
>+      case 'A':
>+              previous_history_commands(out_data);
>+              break;
>+      /* next line */
>+      case 'B':
>+              next_history_commands(out_data);
>+              break;
>+      /* next char */
>+      case 'C':
>+              /* does not support */
>+              break;
>+      /* previous char */
>+      case 'D':
>+              if(out_data->input_mode ==  HISTORY_COMMAND){
>+                      out_data->input_mode=LINE_BUFFER;
>+                      if(history_to_buffer())
>+                              break;
>+              }
>+              previous_char(out_data->stream);
>+              break;
>       }
>-      break;
>-
>-    default:
>-      if (line_buffer_pos >= line_buffer_max) {
>-          char *new_line = ipmi_mem_alloc(line_buffer_max+10+1);
>-          if (!new_line)
>+      out_data->esc_state = INPUT_NORMAL;
>+    }else if(out_data->esc_state == INPUT_NORMAL){
>+      switch(rc){
>+      /* exit ipmish */
>+      case CONTROL('D'):
>+      case CONTROL('d'):
>+              if(line_buffer_pos == 0){
>+                      done = 1;
>+                      evcount = 1; /* Force a newline */
>+              }
>+              break;
>+      /* new line */
>+      case CONTROL('L'):
>+      case CONTROL('l'):
>+              fputc('\n', out_data->stream);
>+              redraw_cmdline(1);
>+              break;
>+      case '\033':
>+              out_data->esc_state = INPUT_PREESC;
>+              break;
>+      case 0x7f: /* delete */
>+      case '\b': /* backspace */
>+              if(out_data->input_mode == HISTORY_COMMAND){
>+                      out_data->input_mode=LINE_BUFFER;
>+                      if(history_to_buffer())
>+                              break;
>+              }
>+              previous_char(out_data->stream);
>+              break;
>+      case '\r':
>+      case '\n':
>+              fputc(rc, out_data->stream);
>+              if(out_data->input_mode==LINE_BUFFER){
>+                      if (line_buffer) {
>+                          line_buffer[line_buffer_pos] = '\0';
>+                          for (i=0; isspace(line_buffer[i]); i++)
>+                              ;
>+                          /* Ignore blank lines. */
>+                          if (line_buffer[i] != '\0') {
>+                              /* Turn off input processing. */
>+                              disable_term_fd(info);
>+      
>+                              cmdlang.err = 0;
>+                              cmdlang.errstr = NULL;
>+                              cmdlang.errstr_dynalloc = 0;
>+                              cmdlang.location = NULL;
>+                              handling_input = 0;
>+                              line_buffer_pos = 0;
>+                              record_history_command(line_buffer);
>+                              ipmi_cmdlang_handle(&cmdlang, line_buffer);
>+                              history_commands.current_idx=-1;
>+                          } else {
>+                              fputs("> ", out_data->stream);
>+                          }   
>+                      } else {
>+                          fputs("> ", out_data->stream);
>+                      }
>+              }else{
>+                      /* Turn off input processing. */
>+                      disable_term_fd(info);
>+
>+                      cmdlang.err = 0;
>+                      cmdlang.errstr = NULL;
>+                      cmdlang.errstr_dynalloc = 0;
>+                      cmdlang.location = NULL;
>+                      handling_input=0;
>+                      line_buffer_pos = 0;
>+                      /* ipmi_cmdlang_handle will modify content of command 
>line, so send a copy to it */
>+                      if(line_buffer){
>+                              ipmi_mem_free(line_buffer);
>+                              line_buffer=NULL;
>+                      }
>+                      
>line_buffer=ipmi_mem_alloc(history_commands.buffers_len[history_commands.current_idx]+1);
>+                      if(!line_buffer){
>+                              fprintf(stderr, "allocate mem failed.\n");
>+                              break;
>+                      }
>+                      
>record_history_command(history_commands.buffers[history_commands.current_idx]);
>+                      memset(line_buffer, 0, 
>history_commands.buffers_len[history_commands.current_idx]+1);
>+                      memcpy(line_buffer, 
>+                              
>history_commands.buffers[history_commands.current_idx], 
>+                              
>history_commands.buffers_len[history_commands.current_idx]+1);
>+                      ipmi_cmdlang_handle(&cmdlang, line_buffer);
>+                      
>line_buffer_max=history_commands.buffers_len[history_commands.current_idx];
>+
>+                      history_commands.current_idx=-1;
>+                      out_data->input_mode=LINE_BUFFER;
>+              }
>+              break;
>+      default :
>+              if(out_data->input_mode == HISTORY_COMMAND){
>+                      out_data->input_mode=LINE_BUFFER;
>+                      if(history_to_buffer())
>+                              break;
>+              }
>+              if(line_buffer_pos > MAX_LINE_BUFFER_SIZE){
>+                      break;
>+              }
>+              
>+              if (line_buffer_pos >= line_buffer_max) {
>+                  char *new_line = ipmi_mem_alloc(line_buffer_max+10+1);
>+                  if (!new_line)
>+                      break;
>+                  line_buffer_max += 10;
>+                  if (line_buffer) {
>+                      memcpy(new_line, line_buffer, line_buffer_pos);
>+                      ipmi_mem_free(line_buffer);
>+                  }
>+                  line_buffer = new_line;
>+              }
>+              line_buffer[line_buffer_pos] = rc;
>+              line_buffer_pos++;
>+              fputc(rc, out_data->stream);
>               break;
>-          line_buffer_max += 10;
>-          if (line_buffer) {
>-              memcpy(new_line, line_buffer, line_buffer_pos);
>-              ipmi_mem_free(line_buffer);
>-          }
>-          line_buffer = new_line;
>       }
>-      line_buffer[line_buffer_pos] = rc;
>-      line_buffer_pos++;
>-      fputc(rc, out_data->stream);
>-      break;
>+    }else{
>+      fprintf(stderr, "unknow esc_state: %2x\n", out_data->esc_state);
>     }
>-
>     fflush(out_data->stream);
> }
> 
>@@ -694,10 +989,21 @@
> static void
> cleanup_term(void)
> {
>+    int i;
>     if (line_buffer) {
>       ipmi_mem_free(line_buffer);
>       line_buffer = NULL;
>     }
>+    for(i=0; i<MAX_CMD_HISTORY_NR; i++){
>+      if(history_commands.buffers[i]){
>+              ipmi_mem_free(history_commands.buffers[i]);
>+              history_commands.buffers[i]=NULL;
>+              history_commands.buffers_len[i]=-1;
>+      }
>+    }
>+    history_commands.newest_idx=-1;
>+    history_commands.oldest_idx=-1;
>+    
>     disable_term_fd(&cmdlang);
> 
>     if (!term_setup)
>
>  
>



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Reply via email to