Skip to content

Commit

Permalink
kernel: remove SyOriginalArgv & SyOriginalArgc
Browse files Browse the repository at this point in the history
These kept references to the `argv` array around. This is
potentially problematic for consumers of libgap API, specifically
when calling `GAP_Initialize` it is not obvious that one has to
keep the `argv` passed to it alive until after GAP completed its
startup.

While we could argument the documentation for `GAP_Initialize`
(which unfortunately currently does exist at all), it seems
better to obviate the need for this.

This patch is a first step towards this. However, several pointers
to `argv` members remain, at least these:
- `SyCompileOutput`
- `SyCompileInput`
- `SyCompileName`
- `SyCompileMagic1`
- `SyRestoring`
  • Loading branch information
fingolfin committed Jan 2, 2025
1 parent f580daf commit 7e749ab
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 18 deletions.
17 changes: 10 additions & 7 deletions src/gap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1052,13 +1052,13 @@ static Obj FuncSHOULD_QUIT_ON_BREAK(Obj self)
** The general idea is to put all kernel-specific info in here, and clean up
** the assortment of global variables previously used
*/
static Obj KernelArgs;

static Obj FuncKERNEL_INFO(Obj self)
{
Obj res = NEW_PREC(0);
UInt r;
Obj tmp;
UInt i;

AssPRec(res, RNamName("GAP_ARCHITECTURE"), MakeImmString(GAPARCH));
AssPRec(res, RNamName("KERNEL_VERSION"), MakeImmString(SyKernelVersion));
Expand All @@ -1073,15 +1073,11 @@ static Obj FuncKERNEL_INFO(Obj self)
AssPRec(res, RNamName("uname"), SyGetOsRelease());

// make command line available to GAP level
tmp = NEW_PLIST_IMM(T_PLIST, 16);
for (i = 0; SyOriginalArgv[i]; i++) {
PushPlist(tmp, MakeImmString(SyOriginalArgv[i]));
}
AssPRec(res, RNamName("COMMAND_LINE"), tmp);
AssPRec(res, RNamName("COMMAND_LINE"), KernelArgs);

// make environment available to GAP level
tmp = NEW_PREC(0);
for (i = 0; environ[i]; i++) {
for (int i = 0; environ[i]; i++) {
Char * p = strchr(environ[i], '=');
if (!p) {
// should never happen...
Expand Down Expand Up @@ -1317,6 +1313,7 @@ static Int InitKernel (
{
// list of exit functions
InitGlobalBag( &WindowCmdString, "src/gap.c:WindowCmdString" );
InitGlobalBag( &KernelArgs, "src/gap.c:KernelArgs" );

// init filters and functions
InitHdlrFuncsFromTable( GVarFuncs );
Expand Down Expand Up @@ -1458,6 +1455,12 @@ void InitializeGap (
// call kernel initialisation
ModulesInitKernel();

// make command line available to GAP level
KernelArgs = NEW_PLIST_IMM(T_PLIST, *pargc);
for (int i = 0; i < *pargc; i++) {
PushPlist(KernelArgs, MakeImmString(argv[i]));
}

#ifdef HPCGAP
InitMainThread();
#endif
Expand Down
2 changes: 0 additions & 2 deletions src/sysopt.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,6 @@ extern Char * SyRestoring;

extern UInt SyInitializing;

extern Char ** SyOriginalArgv;
extern UInt SyOriginalArgc;

/****************************************************************************
**
Expand Down
9 changes: 0 additions & 9 deletions src/system.c
Original file line number Diff line number Diff line change
Expand Up @@ -533,11 +533,6 @@ static const struct optInfo options[] = {
{ 0, "", 0, 0, 0}};


Char ** SyOriginalArgv;
UInt SyOriginalArgc;



void InitSystem (
Int argc,
Char * argv [],
Expand Down Expand Up @@ -601,10 +596,6 @@ void InitSystem (
SyInstallAnswerIntr();
}

// save the original command line for export to GAP
SyOriginalArgc = argc;
SyOriginalArgv = argv;

// scan the command line for options that we have to process in the kernel
// we just scan the whole command line looking for the keys for the options we recognise
// anything else will presumably be dealt with in the library
Expand Down

0 comments on commit 7e749ab

Please sign in to comment.