From fb3866d2bffd81acb647d485fc0dcd29146a784b Mon Sep 17 00:00:00 2001 From: Brian Taylor Date: Mon, 16 Oct 2023 13:19:43 -0700 Subject: [PATCH] Error handling improvements in cfunc.mod. Ensure that d_process.h wiil always respond to version and interface checks sent from sendheader. This is needed so that the pipe reads in sendheader do not cause Windows to hang when the interface version and in/out counts do not match. This hang was the cause of errors not being reported and the Windows gui hanging. Startup and header checks are now detected in cm_d_process, and the simulator will run but with runtime errors since a d_process model cannot be completely instantiated after initial errors. It would be good to find a means of gracefully halting the simulation run. --- examples/xspice/d_process/d_process.h | 30 ++++++++-- examples/xspice/d_process/prog1-4.cir | 6 +- src/xspice/icm/digital/d_process/cfunc.mod | 68 ++++++++++++++-------- 3 files changed, 73 insertions(+), 31 deletions(-) diff --git a/examples/xspice/d_process/d_process.h b/examples/xspice/d_process/d_process.h index face10ab1..fa624de3a 100644 --- a/examples/xspice/d_process/d_process.h +++ b/examples/xspice/d_process/d_process.h @@ -4,6 +4,15 @@ * \author Uros Platise */ +/* +MODIFICATIONS + + 16 October 2023 Brian Taylor + Always send an acknowledgement to cm_d_process to + avoid the pipe read hanging waiting for a response. + +*/ + #ifndef __NGSPICE_D_PROCESS__ #define __NGSPICE_D_PROCESS__ @@ -28,6 +37,7 @@ static inline int d_process_init(int pipein, int pipeout, uint8_t N_din, uint8_t uint8_t version, N_din, N_dout; } __attribute__((packed))header; #endif + int retval = 1; #if defined(_MSC_VER) || defined(__MINGW64__) if (_read(pipein, &header, sizeof(header)) != sizeof(header)) { @@ -35,19 +45,29 @@ static inline int d_process_init(int pipein, int pipeout, uint8_t N_din, uint8_t if (read(pipein, &header, sizeof(header)) != sizeof(header)) { #endif fprintf(stderr, "Error: Incompatible ngspice d_process header size, requiring version %d\n", header.version); - return 0; + header.version = 0; + header.N_din = 0; + header.N_dout = 0; + retval = 0; + } + if (retval == 0) { + goto respond; } if (header.version != D_PROCESS_FORMAT_VERSION || header.N_din != N_din || header.N_dout != N_dout) { fprintf(stderr, "Error: Incompatible ngspice d_process requiring version %d, number of inputs %d expected %d, and outputs %d expected %d.\n", header.version, header.N_din, N_din, header.N_dout, N_dout); - return 0; + header.version = D_PROCESS_FORMAT_VERSION; + header.N_din = N_din; + header.N_dout = N_dout; + retval = 0; } +respond: #if defined(_MSC_VER) || defined(__MINGW64__) - _write(pipeout, &header, sizeof(header)); // acknowledge connection by returning back the same header + _write(pipeout, &header, sizeof(header)); // acknowledge connection by returning back the header #else - write(pipeout, &header, sizeof(header)); // acknowledge connection by returning back the same header + write(pipeout, &header, sizeof(header)); // acknowledge connection by returning back the header #endif - return 1; + return retval; } #endif diff --git a/examples/xspice/d_process/prog1-4.cir b/examples/xspice/d_process/prog1-4.cir index 032a71ba8..4d1e7d25b 100644 --- a/examples/xspice/d_process/prog1-4.cir +++ b/examples/xspice/d_process/prog1-4.cir @@ -34,7 +34,11 @@ ap1_4a [clk2] clk1 reseto [b1 b2 b3 b4] proc4 run edisplay eprvcd clk1 clk2 o1 o2 o3 o4 q1 q2 q3 q4 b1 b2 b3 b4 zeros qzeros reseto > prog1-4.vcd -shell gtkwave prog1-4.vcd --script nggtk.tcl & +if $oscompiled = 1 | $oscompiled = 8 ; MS Windows + shell start gtkwave prog1-4.vcd --script nggtk.tcl +else + shell gtkwave prog1-4.vcd --script nggtk.tcl & +end quit .endc .end diff --git a/src/xspice/icm/digital/d_process/cfunc.mod b/src/xspice/icm/digital/d_process/cfunc.mod index 7fe08d9ae..79cfc06ff 100644 --- a/src/xspice/icm/digital/d_process/cfunc.mod +++ b/src/xspice/icm/digital/d_process/cfunc.mod @@ -111,6 +111,7 @@ typedef unsigned char uint8_t; typedef struct { int pipe_to_child; int pipe_from_child; + unsigned int error_count; uint8_t N_din, N_dout; // number of inputs/outputs bytes Digital_State_t dout_old[256]; // max possible storage to track output changes } Process_t; @@ -135,7 +136,7 @@ static int sendheader(Process_t * process, int N_din, int N_dout) #endif if (N_din > 255 || N_dout > 255) { - cm_message_send("Error: d_process supports max 255 input and output and 255 output signals"); + cm_message_send("ERROR: d_process supports max 255 input and output and 255 output signals"); return 1; } @@ -144,7 +145,7 @@ static int sendheader(Process_t * process, int N_din, int N_dout) #else if (write(process->pipe_to_child, &header, sizeof(header)) == -1) { #endif - cm_message_send("Error: d_process when sending header"); + cm_message_send("ERROR: d_process when sending header"); return 1; } @@ -154,15 +155,15 @@ static int sendheader(Process_t * process, int N_din, int N_dout) #else if (read(process->pipe_from_child, &header, sizeof(header)) != sizeof(header)) { #endif - cm_message_send("Error: d_process didn't respond to the header"); + cm_message_send("ERROR: d_process didn't respond to the header"); return 1; } if (header.version != D_PROCESS_FORMAT_VERSION) { - cm_message_printf("Error: d_process returned invalid version: %d", header.version); + cm_message_printf("ERROR: d_process returned invalid version: %d", header.version); return 1; } if (header.N_din != N_din || header.N_dout != N_dout) { - cm_message_printf("Error: d_process I/O mismatch: in %d vs. returned %d, out %d vs. returned %d", + cm_message_printf("ERROR: d_process I/O mismatch: in %d vs. returned %d, out %d vs. returned %d", N_din, header.N_din, N_dout, header.N_dout); return 1; } @@ -204,7 +205,7 @@ static int dprocess_exchangedata(Process_t * process, double time, uint8_t din[] wlen = write(process->pipe_to_child, &packet, sizeof(double) + process->N_din); #endif if (wlen == -1) { - cm_message_send("Error: d_process when writing exchange data"); + cm_message_send("ERROR: d_process when writing exchange data"); return 1; } @@ -214,7 +215,7 @@ static int dprocess_exchangedata(Process_t * process, double time, uint8_t din[] if (read(process->pipe_from_child, dout, process->N_dout) != process->N_dout) { #endif cm_message_printf( - "Error: d_process received invalid dout count, expected %d", + "ERROR: d_process received invalid dout count, expected %d", process->N_dout); return 1; } @@ -228,10 +229,13 @@ static int start(char *system_command, char * c_argv[], Process_t * process) int pipe_to_child[2]; int pipe_from_child[2]; int pid; - size_t syscmd_len = strlen(system_command); + size_t syscmd_len = 0; + if (system_command) { + syscmd_len = strlen(system_command); + } - if (syscmd_len == 0) { - cm_message_send("Error: d_process process_file argument is not given"); + if (!system_command || syscmd_len == 0) { + cm_message_send("ERROR: d_process process_file argument is not given"); return 1; } if (system_command[syscmd_len-1] == '|') { @@ -245,11 +249,11 @@ static int start(char *system_command, char * c_argv[], Process_t * process) strncpy(filename_out, system_command, syscmd_len-1); strcpy(&filename_out[syscmd_len-1], "_out"); if ((process->pipe_to_child = open(filename_in, O_WRONLY)) == -1) { - cm_message_send("Error: d_process failed to open in fifo"); + cm_message_send("ERROR: d_process failed to open in fifo"); return 1; } if ((process->pipe_from_child = open(filename_out, O_RDONLY)) == -1) { - cm_message_send("Error: d_process failed to open out fifo"); + cm_message_send("ERROR: d_process failed to open out fifo"); return 1; } if (filename_in) free(filename_in); @@ -257,7 +261,7 @@ static int start(char *system_command, char * c_argv[], Process_t * process) } else { if (pipe(pipe_to_child) || pipe(pipe_from_child) || (pid=fork()) ==-1) { - cm_message_send("Error: d_process cannot create pipes and fork"); + cm_message_send("ERROR: d_process cannot create pipes and fork"); return 1; } if (pid == 0) { @@ -268,7 +272,7 @@ static int start(char *system_command, char * c_argv[], Process_t * process) if (execv(system_command, c_argv) == -1) { fprintf(stderr, - "Error: d_process failed to fork or start process %s\n", + "\nERROR: d_process failed to fork or start process %s\n", system_command); exit(1); } @@ -303,6 +307,7 @@ static void cm_d_process_callback(ARGS, Mif_Callback_Reason_t reason) void cm_d_process(ARGS) { int i; /* generic loop counter index */ + int err; Digital_State_t *reset, /* storage for reset value */ *reset_old; /* previous reset value */ @@ -343,11 +348,19 @@ void cm_d_process(ARGS) #undef C_ARGV_SIZE #if defined(_MSC_VER) || defined(__MINGW64__) - (void) w_start(c_argv[0], (const char *const *)c_argv, local_process); + err = w_start(c_argv[0], (const char *const *)c_argv, local_process); #else - (void) start(c_argv[0], c_argv, local_process); + err = start(c_argv[0], c_argv, local_process); #endif - (void) sendheader(local_process, PORT_SIZE(in), PORT_SIZE(out)); + if (err) { + local_process->error_count++; + return; + } + err = sendheader(local_process, PORT_SIZE(in), PORT_SIZE(out)); + if (err) { + local_process->error_count++; + return; + } for (i=0; ierror_count > 0) { + return; + } clk = (Digital_State_t *) cm_event_get_ptr(0,0); clk_old = (Digital_State_t *) cm_event_get_ptr(0,1); @@ -446,22 +461,25 @@ static int w_start(char *system_command, const char *const *argv, Process_t * pr int pipe_from_child[2]; intptr_t pid = 0; int mode = _O_BINARY; - size_t syscmd_len = strlen(system_command); + size_t syscmd_len = 0; + if (system_command) { + syscmd_len = strlen(system_command); + } - if (syscmd_len == 0) { - cm_message_send("Error: d_process process_file argument is not given"); + if (!system_command || syscmd_len == 0) { + cm_message_send("ERROR: d_process process_file argument is not given"); return 1; } if (system_command[syscmd_len-1] == '|') { - cm_message_send("Error: d_process named pipe/fifo not supported"); + cm_message_send("ERROR: d_process named pipe/fifo not supported"); return 1; } if (_pipe(pipe_to_child, 1024, mode) == -1) { - cm_message_send("Error: d_process failed to open pipe_to_child"); + cm_message_send("ERROR: d_process failed to open pipe_to_child"); return 1; } if (_pipe(pipe_from_child, 1024, mode) == -1) { - cm_message_send("Error: d_process failed to open pipe_from_child"); + cm_message_send("ERROR: d_process failed to open pipe_from_child"); return 1; } @@ -473,7 +491,7 @@ static int w_start(char *system_command, const char *const *argv, Process_t * pr _flushall(); pid = _spawnvp(_P_NOWAIT, system_command, argv); if (pid == -1) { - cm_message_printf("Error: d_process failed to spawn %s", system_command); + cm_message_printf("ERROR: d_process failed to spawn %s", system_command); return 1; }