Re: [MiNT] XaAES: bug(s) in shel_write()

[Thorsten Otto <halgara@yahoo.de>]

> What do you mean with:

> > i even verified it again when that bug first appeared

I meant i debugged my own code again that constructs the environment with the ARGV parameters.

> What XaAES-version does what you want?

I tried it only with the XaAES that is intregated in MiNT, that is MiNT 1.19 with XaAES 1.6.4

> Where does XaAES parse the ARGV-part of the caller's environment if you 
> meant that.

It doesn't, and that is part of the problem. But it looks at the length byte being 127 (or wiscr being 1). If the length byte is 127, the parameters should *only* be taken from the ARGV environment variable. The contents of the cmdline are irrelevant in this case, maybe they contain the first 126 chars of the parameters, but even that cannot be guaranteed, and they are definitely truncated.

> I guess the purpose SW_ENVIRON is to pass the caller's environment to the 
> callee's without interfering of the AES. You don't need it I think.

Of course i need it. Where else should i put the ARGV environment? 

> The  meaning of "use ARGV" on the other hand is that the AES writes the 
> command-line into the environment for the callee, not that it reads it 
> from there.

That could be, although it means it is impossible to pass parameters with more than 126 chars to the program, which is a rather bad limitation, especially for TOS programs. But then it should fail if the command length is 127 instead of passing wrong or truncated parameters to the program.

BTW: the comment in mt_gem.h reads:

#define CL_PARSE             1       /**< command line passed in ARGV environment string, see mt_shel_write() */

> strings is written in make_argv().

You are right, i misunderstood that code. Looks like strings and C.env are identical in that case. But it still would fail if the caller passes its own enviroment via SW_ENVIRON, and shel_write() builds a new environment (maybe only because constructing the new command line exceeds the limit).

> I have attached my test-programs. Is it this what you mean?

Basically, yes. One major difference though is that your sample code uses mode 0, and since your callee does not have a ".tos" extension, it will be run as GEM application, not using $TOSRUN.

I have attached a modified caller.c, that reflects what i try to do.

Helmut Karlowski <helmut.karlowski@ish.de> schrieb am 0:52 Samstag, 6.Februar 2016:

Am 05.02.2016, 03:04 Uhr, schrieb Thorsten Otto:

>> What is SHD_ENVIRON? I've found
> Its the same as SW_ENVIRON. I took the name from the XaAES sources (the 
> ones that are part of MiNT).
>
>> What exactly did you write in ARGV?
> The arguments that should be passed to the program of course, including 
> the command name. That part works,

Actually I don't think it matters what you put in ARGV.

What do you mean with:

> i even verified it again when that bug first appeared.

What XaAES-version does what you want?

>> Looks like that is not supported by XaAES.
> The code look like it tries to do so, but fails to do it correctly. If

Where does XaAES parse the ARGV-part of the caller's environment if you 
meant that.

> the original arguments are too long to be passed in the basepage, and 
> this fact is mentioned as 127 being used as length, the function should 
> either fail with an error indicating that this is not supported, or 
> support it correctly. It does not make sense to pass wrong, truncated 
> arguments to the program instead, this can be rather disastrous. BTW 
> XaAES does indicate in appl_getinfo(10) that ARGV *is* supported.

I tried with a test-program and it indeed truncated the command-line to 
124 bytes.

When I remove:

      if (longtail > 124)
        longtail = 124;

all arguments are passed to the launched program. That looks wrong.

>> Only if SW_ENVIRON is set.
> But this is the only way to pass the environment in the call that 
> contains the ARGV element. The application itself does not create a new 
> process where this could be done, nor does XaAES has access to the 
> process' environment.

I guess the purpose SW_ENVIRON is to pass the caller's environment to the 
callee's without interfering of the AES. You don't need it I think. The 
meaning of "use ARGV" on the other hand is that the AES writes the 
command-line into the environment for the callee, not that it reads it 
from there.

>> Do you have a simple test-case?
> Not really. It has to be a complete GEM program after all. I can try to 
> make one if it is really needed. But i hope my explanations made clear 
> that something is going wrong there. Not using the environment that was 
> constructed by make_argv() is at least definitely a bug. And that code

I uses it:

        ret = create_process(cmd, t,
                  (x_mode & SW_ENVIRON) ? x_shell.env : *strings,
                  &p, 0, cpopts.mode ? &cpopts : NULL);

strings is written in make_argv().

I have attached my test-programs. Is it this what you mean?

-Helmut

/*caller.c last change: Sat Feb  6 00:48:26 2016*/
#include <stdio.h>
#include <string.h>
#include <gem.h>
#include <osbind.h>

extern char **environ;

#define TOS_ARGS 126

char *make_argv(char cmd[128], const char *const *argv)
{
	size_t cmlen;
	size_t enlen = 0;
	size_t left, min_left;
	const char *p;
	char *s, *t;
	char *env;
	const char *const *envp;

	envp = (const char *const *)environ;

/* count up space needed for environment */
	for (cmlen = 0; argv[cmlen]; cmlen++)
		enlen += strlen(argv[cmlen]) + 1;
	enlen += 64;						/* filler for stuff like ARGV= and zeros, 
										 * minibuffer for empty param index conversion 
										 */
	min_left = enlen;
	for (cmlen = 0; envp[cmlen]; cmlen++)
		enlen += strlen(envp[cmlen]) + 1;
	enlen += 1024;						/* buffer for _unx2dos */

  try_again:
	if ((env = (char *) Malloc((long) enlen)) == NULL)
	{
		return NULL;
	}

	left = enlen;
	s = env;

	while ((p = *envp) != 0)
	{
		/* copy variable without any conversion */
		while (*p)
		{
			*s++ = *p++;
			if (--left <= min_left)
			{
				/* oh dear, we don't have enough core...
				 * so we Mfree what we already have, and try again with
				 * some more space.
				 */
				Mfree(env);
				enlen += 1024;
				goto try_again;
			}
		}
		*s++ = 0;
		left--;

		envp++;
	}

	strcpy(s, "ARGV=");
	s += 6;								/* s+=sizeof("ARGV=") */

	if (argv && *argv)
	{
		/* copy argv[0] first (because it doesn't go into 
		 * the command line)
		 */
		p = *argv;
		do
		{
			*s++ = *p++;
		} while (*p);
		*s++ = '\0';
	}

	t = cmd;
	memset(t, 0, TOS_ARGS + 2);

/* s points at the environment's copy of the args */
/* t points at the command line copy to be put in the basepage */

	cmlen = 0;
	if (argv && *argv)
	{
		t++;
		while (*++argv)
		{
			p = *argv;
			do
			{
				if (cmlen < TOS_ARGS)
				{
					*t++ = *p;
					cmlen++;
				}
				*s++ = *p++;
			} while (*p);
			if (cmlen < TOS_ARGS && *(argv + 1))
			{
				*t++ = ' ';
				cmlen++;
			}
			*s++ = '\0';
		}
	}

	/* tie off environment */
	*s++ = '\0';
	*s = '\0';

	/* signal Extended Argument Passing */
	*cmd = 0x7f;
	
	return env;
}



int main(void)
{
	int rtrn	= 0;
	char cmd[128];
	const char *argv[130];
	
	if ((appl_init()) != -1)
	{
		char buffer[4096];
		int i, c = 0, argc = 0;
		char *env;
		
		struct {
			const char *newcmd;
			long psetlimit;
			long prenice;
			const char *defdir;
			char *env;
			short uid;
			short gid;
		} x_shell;
		
		argv[argc++] = "callee.tos";
		for( i = 0; i < 20; i++ )
		{
			argv[argc++] = buffer + c;
			c += sprintf( buffer+c, "argument%d", i ) + 1;
		}
		argv[argc] = NULL;
		env = make_argv(cmd, argv);

		x_shell.newcmd = argv[0];
		x_shell.psetlimit = 0;
		x_shell.prenice = 0;
		x_shell.defdir = 0;
		x_shell.env = env;
		x_shell.uid = 0;
		x_shell.gid = 0;

		shel_write(SWM_LAUNCHNOW|SW_ENVIRON, 0, CL_PARSE, &x_shell, cmd);

		appl_exit();
	}
	else {
		rtrn = 1;
	}
	return rtrn;
}
/* -- end of caller.c -- */