[GRASS-dev] Re: testing native winGRASS

[Glynn Clements]

Moritz Lennert wrote:

> >> general/g.mapset/main.c:135:    ret = system ( buf ) ;
> >> G_asprintf ( &buf, "%s %s/.gislock %s", lock_prog, mapset_new_path, gis_lock );
> > 
> > 	sprintf(path, "%s/.gislock", mapset_new_path);
> > 	G_spawn(lock_prog, lock_prog, path, gis_lock, NULL);
> 
> Does the return value of G_spawn have the same meaning than the one of 
> system and can thus be used in the same way (i.e. in this case:
> 
> if ( ret != 0 )
>          G_fatal_error ( "%s is currently running GRASS in selected
>                 mapset or lock file cannot be checked.", G_whoami());

Yes. system(), G_system() and G_spawn() all return the exit status of
the program.

> >> general/g.mapset/main.c:165:    system( buf );
> >> G_asprintf ( &buf, "/bin/sh -c \"%s/etc/clean_temp > /dev/null\"", G_gisbase() );
> > 
> > 	sprintf(path, "%s/etc/clean_temp", G_gisbase());
> > 	G_spawn(path, "clean_temp", NULL);
> > or:
> > 	G_spawn_ex(path, "clean_temp", SF_REDIRECT_FILE, 1, O_WRONLY, "/dev/null", NULL);
> 
> G_spawn_ex is not ready for windows, yet, correct ?

Correct.

> Should I wait until this is done, or just go ahead with implementing the 
> first solution ?

I'm not sure. It would be nice if someone would volunteer to write the
Windows implementation (using CreateProcess()).

Hmm. We might want a function which returns the name of the platform's
null device (i.e. /dev/null on Unix, NUL: on Windows).

> Just for my education: why is there a choice here and not in the 
> preceding case ?

In the preceding case, there's no benefit to using G_spawn_ex(). In
this case, using G_spawn() means that you lose the redirection to
/dev/null.

> >> general/g.mapsets/set_path.c:65:    if (system (command) == 0)
> >> strcpy (command, "g.mapsets -p mapset=") & other strcat calls
> > 
> > 	sprintf(mapset_arg, "mapset=%s", mapset)
> > 	G_spawn("g.mapsets", "g.mapsets", "-p", mapset_arg, <other args>, NULL);
> 
> As the different strcat calls are embedded in if-then checks, I imagine 
> that we have to keep these checks and construct an array of <other args> ?

Ah. That will need to wait until G_spawn_ex() has been extended (or
has a G_vspawn_ex() version).

> >> lib/gis/gisbase.c:34:  system (command);
> >> sprintf (command, "%s/etc/sroff", G_gisbase( ) );
> > 
> > 	G_spawn(command, "sroff", NULL);
> 
> I just realised that this is actually a comment to explain how to use 
> G_gisbase(). Should this example be replaced by another one, not to 
> incite people to use system() ?

Yes please.

> >> lib/gis/gishelp.c:55:	system(buffer) ;
> >> sprintf(buffer, "%%GRASS_PAGER%% %s", file) ; /*ifdef __MINGW32__*/
> >> sprintf(buffer, "$GRASS_PAGER %s", file) ;/*else*/
> > 
> > 	G_spawn(getenv("GRASS_PAGER"), getenv("GRASS_PAGER"), file, NULL);
> 
> done, but I get:
> 
> warning: implicit declaration of function 'G_spawn'
> 
> Is this a problem ? If yes, do I need to add an include statement or 
> change something in the make file ?

Currently:

	#include <grass/spawn.h>

Maybe it could be merged with gis.h/gisdefs.h.

> >> lib/imagery/ls_groups.c:70&129:    G_system(buf);
> >> sprintf (buf, "$GRASS_PAGER %s", tempfile);
> > 
> > 	G_spawn(getenv("GRASS_PAGER"), getenv("GRASS_PAGER"), tempfile, NULL);
> 
> done
> 
> A few lines after the G_system(buf) call, there is a G_gets(buf) call. 
> What does this do ?

Re-using the same buffer for different things.

> Can I erase that ?

No.

> I also see the following code in ls_groups.c (line 37ff):
> 
> strcpy (buf, "cd ");
> G__file_name (buf+strlen(buf), element, "", G_mapset());
> strcat (buf, ";ls");
> if (!full) strcat (buf, " -C");
> if ((ls = popen (buf, "r")))

This should probably use G__ls().

> Isn't this somewhat equivalent to a system() call ?

popen() is similar to system, in that it spawns a command using a
shell. But popen() returns a FILE* corresponding to either the
program's stdin ("w") or stdout ("r"), and doesn't wait for the
command to complete.

You can achieve the same thing using G_spawn_ex(), pipe() and
fdopen(), e.g.:

	int fds[2];
	FILE *fp;
	pid_t pid;

	pipe(fds);
	pid = G_spawn_ex(..., SF_BACKGROUND, SF_REDIRECT_DESCRIPTOR, 1, fds[1], SF_CLOSE_DESCRIPTOR, fds[0], ...);
	close(fds[1]);
	fp = fdopen(fds[0], "r");

> >> lib/init/mke_loc.c:179:    system(buf);
> >> sprintf (buf, "echo %s >  \"%s/%s/%s/MYNAME\"", myname, gisdbase, location_name, mapset);  &  G_convert_dirseps_to_host(buf);
> > 
> > 	sprintf(path, "%s/%s/%s/MYNAME", gisdbase, location_name, mapset);
> 
> No need to run G_convert_dirseps_to_host(path) here ?

No; you only need that function if you are passing the filename to a
command; Windows system calls can handle / as the separator, but
commands treat it as signifying an option (analogous to - on Unix).

> >> raster/r.coin/inter.c:32:    G_system("clear");
> >>
> >> raster/r.coin/inter.c:51:	G_system("clear");
> > 
> > 	G_spawn("clear", "clear", NULL);
> 
> Should be G_clear_screen(). I guess I can just go ahead with this and we 
> can deal with the internals of G_clear_screen() later, or ?

Yes.

> Any rule as to how I should declare path (char path[254] ?), i.e. which 
> length ?

GPATH_MAX. It's currently hard-coded to 4096, which is likely to be
sufficient (that's over 50 80-column lines; no-one is going to have a
$HOME directory anywhere near that long).

-- 
Glynn Clements <glynn at gclements.plus.com>