From b45dc64441703e9e0fa72b1339f5e08087dd460a Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 14 Jun 2017 15:04:49 +0200 Subject: [PATCH 1/6] READ_GAP_ROOT: reduce code duplication, cleanup --- src/streams.c | 41 +++++++++-------------------------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/src/streams.c b/src/streams.c index 6e8c1b0a6e..68937d15e2 100644 --- a/src/streams.c +++ b/src/streams.c @@ -438,31 +438,11 @@ Int READ_GAP_ROOT ( Char * filename ) return 0; } - /* dynamically linked */ - else if ( res == 1 ) { + /* dynamically or statically linked */ + else if ( res == 1 || res == 2 ) { if ( SyDebugLoading ) { - Pr( "#I READ_GAP_ROOT: loading '%s' dynamically\n", - (Int)filename, 0L ); - } - info = result.module_info; - res = info->initKernel(info); - if (!SyRestoring) { - UpdateCopyFopyInfo(); - res = res || info->initLibrary(info); - } - if ( res ) { - Pr( "#W init functions returned non-zero exit code\n", 0L, 0L ); - } - - RecordLoadedModule(info, 1, filename); - return 1; - } - - /* statically linked */ - else if ( res == 2 ) { - if ( SyDebugLoading ) { - Pr( "#I READ_GAP_ROOT: loading '%s' statically\n", - (Int)filename, 0L ); + const char *s = (res == 1) ? "dynamically" : "statically"; + Pr( "#I READ_GAP_ROOT: loading '%s' %s\n", (Int)filename, (Int)s ); } info = result.module_info; res = info->initKernel(info); @@ -479,18 +459,15 @@ Int READ_GAP_ROOT ( Char * filename ) /* special handling for the other cases, if we are trying to load compiled modules needed for a saved workspace ErrorQuit is not available */ - else if (SyRestoring) - { - if (res == 3 || res == 4) - { + else if (SyRestoring) { + if (res == 3 || res == 4) { Pr("Can't find compiled module '%s' needed by saved workspace\n", (Int) filename, 0L); return 0; - } - else - Pr("unknown result code %d from 'SyFindGapRoot'", res, 0L ); + } + Pr("unknown result code %d from 'SyFindGapRoot'", res, 0L ); SyExit(1); - } + } /* ordinary gap file */ else if ( res == 3 || res == 4 ) { From 51569af7673e75c232060038a89aae1dae6f0fa4 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 14 Jun 2017 16:35:51 +0200 Subject: [PATCH 2/6] Remove CLI option -X and obsolete CRC checking code This also removes the global variable SyCheckCRCCompiledModule. Note: This does not remove the CRC checks for GAP files compiled to C cod by GAC. Rather, the removed code was a left-over from the long-gone "completion" mechanism. It did not actually do anything, because the only call to SyFindOrLinkGapRootFile hardcoded 0 as reference CRC. --- doc/ref/run.xml | 7 ------- hpcgap/lib/system.g | 1 - lib/system.g | 1 - src/streams.c | 6 +++--- src/sysfiles.c | 26 ++++++-------------------- src/sysfiles.h | 4 +--- src/system.c | 9 --------- src/system.h | 7 ------- 8 files changed, 10 insertions(+), 51 deletions(-) diff --git a/doc/ref/run.xml b/doc/ref/run.xml index 99f3c852ee..10d3529980 100644 --- a/doc/ref/run.xml +++ b/doc/ref/run.xml @@ -384,13 +384,6 @@ suppresses the usual break loop behaviour of ⪆. With this option This is intended for automated testing of ⪆. This option may be repeated to toggle this behavior on and off. --X --X - -tells ⪆ to do a consistency check of the library file and the -corresponding compiled module when loading the compiled module. This -option may be repeated to toggle this behavior on and off. - -x -x length diff --git a/hpcgap/lib/system.g b/hpcgap/lib/system.g index 061ea7366a..0b46d03d16 100644 --- a/hpcgap/lib/system.g +++ b/hpcgap/lib/system.g @@ -93,7 +93,6 @@ BIND_GLOBAL( "GAPInfo", AtomicRecord(rec( rec( short:= "M", default := false, help := ["disable/enable loading of compiled modules"] ), rec( short:= "N", default := false, help := ["unused, for backward compatibility only"] ), rec( short:= "O", default := false, help := ["disable/enable loading of obsolete files"] ), - rec( short:= "X", default := false, help := ["enable/disable CRC checking for compiled modules"] ), rec( short:= "T", default := false, help := ["disable/enable break loop"] ), rec( long := "quitonbreak", default := false, help := ["quit GAP with non-zero return value instead of entering break loop"]), , diff --git a/lib/system.g b/lib/system.g index bce6e60bab..066ca7af7f 100644 --- a/lib/system.g +++ b/lib/system.g @@ -92,7 +92,6 @@ BIND_GLOBAL( "GAPInfo", rec( rec( short:= "M", default := false, help := ["disable/enable loading of compiled modules"] ), rec( short:= "N", default := false, help := ["unused, for backward compatibility only"] ), rec( short:= "O", default := false, help := ["disable/enable loading of obsolete files"] ), - rec( short:= "X", default := false, help := ["enable/disable CRC checking for compiled modules"] ), rec( short:= "T", default := false, help := ["disable/enable break loop"] ), rec( long := "quitonbreak", default := false, help := ["quit GAP with non-zero return value instead of entering break loop"]), , diff --git a/src/streams.c b/src/streams.c index 68937d15e2..5020900e42 100644 --- a/src/streams.c +++ b/src/streams.c @@ -431,7 +431,7 @@ Int READ_GAP_ROOT ( Char * filename ) StructInitInfo * info; /* try to find the file */ - res = SyFindOrLinkGapRootFile( filename, 0L, &result ); + res = SyFindOrLinkGapRootFile( filename, &result ); /* not found */ if ( res == 0 ) { @@ -460,7 +460,7 @@ Int READ_GAP_ROOT ( Char * filename ) /* special handling for the other cases, if we are trying to load compiled modules needed for a saved workspace ErrorQuit is not available */ else if (SyRestoring) { - if (res == 3 || res == 4) { + if ( res == 3 ) { Pr("Can't find compiled module '%s' needed by saved workspace\n", (Int) filename, 0L); return 0; @@ -470,7 +470,7 @@ Int READ_GAP_ROOT ( Char * filename ) } /* ordinary gap file */ - else if ( res == 3 || res == 4 ) { + else if ( res == 3 ) { if ( SyDebugLoading ) { Pr( "#I READ_GAP_ROOT: loading '%s' as GAP file\n", (Int)filename, 0L ); diff --git a/src/sysfiles.c b/src/sysfiles.c index 3b8f4759b8..8f867a05b4 100644 --- a/src/sysfiles.c +++ b/src/sysfiles.c @@ -109,7 +109,7 @@ ssize_t writeandcheck(int fd, const char *buf, size_t count) { /**************************************************************************** ** -*F SyFindOrLinkGapRootFile( , , ) . . . . load or link +*F SyFindOrLinkGapRootFile( , ) . . . . . . load or link ** ** 'SyFindOrLinkGapRootFile' tries to find a GAP file in the root area and ** check if there is a corresponding statically or dynamically linked @@ -122,17 +122,14 @@ ssize_t writeandcheck(int fd, const char *buf, size_t count) { ** 1: if a dynamically linked module was found ** 2: if a statically linked module was found ** 3: a GAP file was found -** 4: a GAP file was found and the CRC value didn't match */ #include /* statically linked modules */ Int SyFindOrLinkGapRootFile ( const Char * filename, - Int4 crc_gap, TypGRF_Data * result ) { - UInt4 crc_sta = 0; Int found_gap = 0; Int found_sta = 0; Char module[GAP_PATH_MAX]; @@ -152,7 +149,6 @@ Int SyFindOrLinkGapRootFile ( /* try to find any statically link module */ strxcpy( module, "GAPROOT/", sizeof(module) ); - strxcat( module, filename, sizeof(module) ); for ( k = 0; CompInitFuncs[k]; k++ ) { info_sta = (*(CompInitFuncs[k]))(); @@ -160,28 +156,18 @@ Int SyFindOrLinkGapRootFile ( continue; } if ( ! strcmp( module, info_sta->name ) ) { - crc_sta = info_sta->crc; found_sta = 1; break; } } - /* check if we have to compute the crc */ - if ( found_gap && ( found_sta ) ) { - if ( crc_gap == 0 ) { - crc_gap = SyGAPCRC(result->pathname); - } else if ( SyCheckCRCCompiledModule ) { - if ( crc_gap != SyGAPCRC(result->pathname) ) { - return 4; - } + /* if there is both a GAP and a statically linked module, check CRC */ + if ( found_gap && found_sta ) { + if ( info_sta->crc != SyGAPCRC(result->pathname) ) { + Pr("#W Static module %s has CRC mismatch, ignoring\n", (Int)filename, 0); + found_sta = 0; } } - - /* now decide what to do */ - if ( found_gap && found_sta && crc_gap != crc_sta ) { - Pr("#W Static module %s has CRC mismatch, ignoring\n", (Int) filename, 0); - found_sta = 0; - } if ( found_gap && found_sta ) { result->module_info = info_sta; return 2; diff --git a/src/sysfiles.h b/src/sysfiles.h index 3da14d71ed..333cd4da32 100644 --- a/src/sysfiles.h +++ b/src/sysfiles.h @@ -30,7 +30,7 @@ /**************************************************************************** ** -*F SyFindOrLinkGapRootFile( , , ) . . . . load or link +*F SyFindOrLinkGapRootFile( , ) . . . . . . load or link ** ** 'SyFindOrLinkGapRootFile' tries to find a GAP file in the root area and ** check if there is a corresponding statically or dynamically linked @@ -43,7 +43,6 @@ ** 1: if a dynamically linked module was found ** 2: if a statically linked module was found ** 3: a GAP file was found -** 4: a GAP file was found and the CRC value didn't match */ typedef union { @@ -53,7 +52,6 @@ typedef union { extern Int SyFindOrLinkGapRootFile ( const Char * filename, - Int4 crc_gap, TypGRF_Data * result ); diff --git a/src/system.c b/src/system.c index cb0c44539f..09159f198a 100644 --- a/src/system.c +++ b/src/system.c @@ -126,13 +126,6 @@ const Char * SyArchitecture = SYS_ARCH; UInt SyCTRD; -/**************************************************************************** -** -*V SyCheckCRCCompiledModule . . . check crc while loading compiled modules -*/ -Int SyCheckCRCCompiledModule; - - /**************************************************************************** ** *V SyCompileInput . . . . . . . . . . . . . . . . . . from this input file @@ -1727,7 +1720,6 @@ struct optInfo options[] = { { 'K', "maximal-workspace", storeMemory2, &SyStorKill, 1}, /* could handle from library with new interface */ { 'L', "", storeString, &SyRestoring, 1}, /* must be handled in kernel */ { 'M', "", toggle, &SyUseModule, 0}, /* must be handled in kernel */ - { 'X', "", toggle, &SyCheckCRCCompiledModule, 0}, /* must be handled in kernel */ { 'R', "", unsetString, &SyRestoring, 0}, /* kernel */ { 'a', "", storeMemory, &preAllocAmount, 1 }, /* kernel -- is this still useful */ { 'e', "", toggle, &SyCTRD, 0 }, /* kernel */ @@ -1770,7 +1762,6 @@ void InitSystem ( /* Initialize global and static variables. Do it here rather than with initializers to allow for restart */ SyCTRD = 1; - SyCheckCRCCompiledModule = 0; SyCompilePlease = 0; SyDebugLoading = 0; SyHasUserHome = 0; diff --git a/src/system.h b/src/system.h index f257290597..5090793fa3 100644 --- a/src/system.h +++ b/src/system.h @@ -211,13 +211,6 @@ extern const Char * SyBuildDateTime; extern UInt SyCTRD; -/**************************************************************************** - ** - *V SyCheckCRCCompiledModule . . . check crc while loading compiled modules - */ -extern Int SyCheckCRCCompiledModule; - - /**************************************************************************** ** *V SyCompileInput . . . . . . . . . . . . . . . . . . from this input file From 6ce5bde710849f4fed42d57739e00765999f3f28 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 14 Jun 2017 16:37:31 +0200 Subject: [PATCH 3/6] kernel: remove some empty lines inside comments --- src/gvars.h | 5 ----- src/sysfiles.c | 2 -- src/sysfiles.h | 12 ------------ 3 files changed, 19 deletions(-) diff --git a/src/gvars.h b/src/gvars.h index 9a1f6d5b96..193df005cf 100644 --- a/src/gvars.h +++ b/src/gvars.h @@ -187,14 +187,12 @@ extern Int IsReadOnlyGVar ( /**************************************************************************** ** - *F * * * * * * * * * * * * * copies and fopies * * * * * * * * * * * * * * * */ /**************************************************************************** ** - *F InitCopyGVar( , ) . . declare C variable as copy of global ** ** 'InitCopyGVar' makes the C variable at address a copy of @@ -300,14 +298,12 @@ extern void SetGVar(GVarDescriptor *gvar, Obj obj); /**************************************************************************** ** - *F * * * * * * * * * * * * * initialize package * * * * * * * * * * * * * * * */ /**************************************************************************** ** - *F InitInfoGVars() . . . . . . . . . . . . . . . . . table of init functions */ StructInitInfo * InitInfoGVars ( void ); @@ -317,6 +313,5 @@ StructInitInfo * InitInfoGVars ( void ); /**************************************************************************** ** - *E gvars.h . . . . . . . . . . . . . . . . . . . . . . . . . . . . ends here */ diff --git a/src/sysfiles.c b/src/sysfiles.c index 8f867a05b4..03b4fbf01f 100644 --- a/src/sysfiles.c +++ b/src/sysfiles.c @@ -101,8 +101,6 @@ ssize_t writeandcheck(int fd, const char *buf, size_t count) { /**************************************************************************** ** - - *F * * * * * * * * * * * * * * dynamic loading * * * * * * * * * * * * * * * */ diff --git a/src/sysfiles.h b/src/sysfiles.h index 333cd4da32..d42c4ac095 100644 --- a/src/sysfiles.h +++ b/src/sysfiles.h @@ -22,8 +22,6 @@ /**************************************************************************** ** - - *F * * * * * * * * * * * * * * dynamic loading * * * * * * * * * * * * * * * */ @@ -80,7 +78,6 @@ extern Int SyLoadModule( const Char * name, InitInfoFunc * func ); /**************************************************************************** ** - *F * * * * * * * * * * * * * * * window handler * * * * * * * * * * * * * * * */ @@ -117,8 +114,6 @@ extern Char * SyWinCmd ( /**************************************************************************** ** - - *F * * * * * * * * * * * * * * * * open/close * * * * * * * * * * * * * * * * */ @@ -337,7 +332,6 @@ extern UInt SyIsIntr ( void ); /**************************************************************************** ** - *F * * * * * * * * * * * * * * * * * output * * * * * * * * * * * * * * * * * */ @@ -354,7 +348,6 @@ extern Int SyEchoch ( /**************************************************************************** ** - *F * * * * * * * * * * * * * * * * * input * * * * * * * * * * * * * * * * * */ @@ -390,7 +383,6 @@ extern Int SyGetch ( /**************************************************************************** ** - *F * * * * * * * * * * * * system error messages * * * * * * * * * * * * * * */ @@ -425,7 +417,6 @@ extern void SySetErrorNo ( void ); /**************************************************************************** ** - *F * * * * * * * * * * * * * file and execution * * * * * * * * * * * * * * * */ @@ -546,7 +537,6 @@ extern Char *SyFindGapRootFile(const Char *filename, Char *buffer, size_t buffer /**************************************************************************** ** - *F * * * * * * * * * * * * * * * directories * * * * * * * * * * * * * * * * */ @@ -628,7 +618,6 @@ extern Obj SyReadStringFileGeneric(Int fid); /**************************************************************************** ** - *F * * * * * * * * * * * * * initialize package * * * * * * * * * * * * * * * */ @@ -644,6 +633,5 @@ StructInitInfo * InitInfoSysFiles ( void ); /**************************************************************************** ** - *E sysfiles.h . . . . . . . . . . . . . . . . . . . . . . . . . . ends here */ From 8f5fe99dfe50d624aef83476ab5e8687bd26b313 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 14 Jun 2017 16:45:08 +0200 Subject: [PATCH 4/6] kernel: make syCcitt32 array const --- src/sysfiles.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sysfiles.c b/src/sysfiles.c index 03b4fbf01f..8bef1b9493 100644 --- a/src/sysfiles.c +++ b/src/sysfiles.c @@ -190,7 +190,7 @@ Int SyFindOrLinkGapRootFile ( ** ** This function *never* returns a 0 unless an error occurred. */ -static UInt4 syCcitt32[ 256 ] = +static const UInt4 syCcitt32[ 256 ] = { 0x00000000L, 0x77073096L, 0xee0e612cL, 0x990951baL, 0x076dc419L, 0x706af48fL, 0xe963a535L, 0x9e6495a3L, 0x0edb8832L, 0x79dcb8a4L, 0xe0d5e91eL, From 19a3ee48569b20797ee09b7230bd1dfdb571f820 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 14 Jun 2017 16:47:21 +0200 Subject: [PATCH 5/6] kernel: add READ_GAP_ROOT comment comparing to LOAD_DYN/LOAD_STAT --- src/streams.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/streams.c b/src/streams.c index 5020900e42..5ad131ea0e 100644 --- a/src/streams.c +++ b/src/streams.c @@ -440,6 +440,9 @@ Int READ_GAP_ROOT ( Char * filename ) /* dynamically or statically linked */ else if ( res == 1 || res == 2 ) { + // This code section covers loading of GAC compiled code; in contrast + // to FuncLOAD_STAT and FuncLOAD_DYN, which are typically used by + // kernel extensions to load C/C++ code. if ( SyDebugLoading ) { const char *s = (res == 1) ? "dynamically" : "statically"; Pr( "#I READ_GAP_ROOT: loading '%s' %s\n", (Int)filename, (Int)s ); From 025cc4524387042188961087abbe4be404eeb3d3 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 14 Jun 2017 21:58:01 +0200 Subject: [PATCH 6/6] kernel: reduce diffs between LOAD_DYN and LOAD_STAT --- src/gap.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/gap.c b/src/gap.c index 51af5d1f97..d601137cf4 100644 --- a/src/gap.c +++ b/src/gap.c @@ -1484,10 +1484,10 @@ Obj FuncLOAD_DYN ( Obj filename, Obj crc ) { - InitInfoFunc init; StructInitInfo * info; Obj crc1; Int res; + InitInfoFunc init; /* check the argument */ while ( ! IsStringConv( filename ) ) { @@ -1496,7 +1496,7 @@ Obj FuncLOAD_DYN ( (Int)TNAM_OBJ(filename), 0L, "you can replace via 'return ;'" ); } - while ( ! IS_INTOBJ(crc) && crc!=False ) { + while ( ! IS_INTOBJ(crc) && crc != False ) { crc = ErrorReturnObj( " must be a small integer or 'false' (not a %s)", (Int)TNAM_OBJ(crc), 0L, @@ -1569,8 +1569,8 @@ Obj FuncLOAD_STAT ( { StructInitInfo * info = 0; Obj crc1; - Int k; Int res; + Int k; /* check the argument */ while ( ! IsStringConv( filename ) ) { @@ -1579,7 +1579,7 @@ Obj FuncLOAD_STAT ( (Int)TNAM_OBJ(filename), 0L, "you can replace via 'return ;'" ); } - while ( !IS_INTOBJ(crc) && crc!=False ) { + while ( ! IS_INTOBJ(crc) && crc != False ) { crc = ErrorReturnObj( " must be a small integer or 'false' (not a %s)", (Int)TNAM_OBJ(crc), 0L, @@ -1589,10 +1589,7 @@ Obj FuncLOAD_STAT ( /* try to find the module */ for ( k = 0; CompInitFuncs[k]; k++ ) { info = (*(CompInitFuncs[k]))(); - if ( info == 0 ) { - continue; - } - if ( ! strcmp( CSTR_STRING(filename), info->name ) ) { + if ( info && ! strcmp( CSTR_STRING(filename), info->name ) ) { break; } } @@ -1622,6 +1619,7 @@ Obj FuncLOAD_STAT ( /* link and init me */ res = (info->initKernel)(info); UpdateCopyFopyInfo(); + /* Start a new executor to run the outer function of the module in global context */ ExecBegin( STATE(BottomLVars) );