Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#714 closed defect (fixed)

spawn.c differences between 6.4 and 6.5

Reported by: neteler Owned by: grass-dev@…
Priority: critical Milestone: 6.4.0
Component: Default Version: 6.4.0 RCs
Keywords: Cc:
CPU: Unspecified Platform: MSWindows XP

Description

Comparing lib/gis/spawn.c between 6.4 and 6.5, I found differences. Which is the desired way for 6.4? Diff attached.

Markus

Attachments (1)

spawn.c (24.0 KB) - added by neteler 10 years ago.
New spawn backported from 6.5

Download all attachments as: .zip

Change History (20)

comment:1 in reply to:  description Changed 10 years ago by glynn

Replying to neteler:

Comparing lib/gis/spawn.c between 6.4 and 6.5, I found differences. Which is the desired way for 6.4? Diff attached.

Microsoft's documentation says that the _spawn* functions take "const char *" arguments:

http://msdn.microsoft.com/en-us/library/275khfab(VS.80).aspx

comment:2 Changed 10 years ago by neteler

Milestone: 6.4.06.4.1
Platform: UnspecifiedMSWindows XP

We can backport this in 6.4.1.

comment:3 Changed 10 years ago by (none)

Milestone: 6.4.1

Milestone 6.4.1 deleted

comment:4 Changed 10 years ago by hamish

Milestone: 6.4.1

comment:5 Changed 10 years ago by neteler

spawn.c in GRASS 6.5 has been sync'ed to 7.0 [1]. After some testing, this should be backported to 6.4.1.

[1] http://lists.osgeo.org/pipermail/grass-dev/2010-January/048232.html

comment:6 Changed 10 years ago by hamish

is this anything to worry about?

[...]svn/grass65/dist.x86_64-unknown-linux-gnu/include -o OBJ.x86_64-unknown-linux-gnu/spawn.o -c spawn.c
spawn.c: In function 'parse_argvec':
spawn.c:721: warning: cast from pointer to integer of different size
spawn.c:724: warning: cast from pointer to integer of different size
spawn.c:730: warning: cast from pointer to integer of different size
spawn.c:731: warning: cast from pointer to integer of different size
spawn.c:737: warning: cast from pointer to integer of different size
spawn.c:744: warning: cast from pointer to integer of different size
spawn.c:745: warning: cast from pointer to integer of different size
spawn.c:746: warning: cast from pointer to integer of different size

Hamish

comment:7 in reply to:  6 Changed 10 years ago by glynn

Replying to hamish:

is this anything to worry about?

[...]svn/grass65/dist.x86_64-unknown-linux-gnu/include -o OBJ.x86_64-unknown-linux-gnu/spawn.o -c spawn.c
spawn.c: In function 'parse_argvec':
spawn.c:721: warning: cast from pointer to integer of different size

No. In that context, the pointer is actually an "int" which has been cast to a pointer, so casting it back to an "int" is safe.

If you want to shut the compiler up, you could try changing the definition of the NEXT_ARG macro to:

#define NEXT_ARG(var, type) ((type) (ssize_t) *(var)++)

Changed 10 years ago by neteler

Attachment: spawn.c added

New spawn backported from 6.5

comment:8 Changed 10 years ago by neteler

Milestone: 6.4.16.4.0

I have attached the new spawn.c backported from 6.5 with Glynn's fix from comment #6 included.

Submit this to SVN? I wonder how much non-Windows systems will be affected.

Markus

comment:9 in reply to:  8 ; Changed 10 years ago by hamish

Replying to neteler:

I have attached the new spawn.c backported from 6.5 with Glynn's fix from comment:6 included.

(spawn.c.diff)

Submit this to SVN? I wonder how much non-Windows systems will be affected.

as grass65 and trunk are currently in sync and apparently functional I wouldn't expect too many problems by bringing the third branch in line with the other two.

my vote: this has to happen, so go for it. get the infrastructure functions solid first, and only then worry about sweeping up the little glitches. otherwise we spend all out time patching cracks caused by the old incomplete version.

(and of course we should port that #define to the dev branches too)

thanks, Hamish

comment:10 Changed 10 years ago by hamish

Priority: normalcritical

comment:11 in reply to:  9 Changed 10 years ago by neteler

Replying to hamish:

as grass65 and trunk are currently in sync and apparently functional I wouldn't expect too many problems by bringing the third branch in line with the other two.

my vote: this has to happen, so go for it. ...

Done so: r40644.

(and of course we should port that #define to the dev branches too)

Also done.

In 19hs from now the next winGRASS package should be ready for testing on "josef".

Markus

comment:12 Changed 10 years ago by neteler

Tested and working. "g.gui wxpython" works with GRASS being installed in "Programs and Files".

comment:13 Changed 10 years ago by neteler

Resolution: fixed
Status: newclosed

comment:14 Changed 10 years ago by hamish

shell script modules still not working in the jan 27th nightly build, but it may be a msys.bat thing.

comment:15 Changed 10 years ago by glynn

I've modified g.parser in 6.5 to use G_spawn() instead of _spawnlp(), and that seems to have solved the issue with shell scripts.

comment:16 Changed 10 years ago by neteler

Resolution: fixed
Status: closedreopened

Backported to 6.4 (can be tested in new winGRASS in 1.hs from now) except for

@@ -1989,7 +1998,7 @@

    sprintf(script, "%s/etc/wxpython/gui_modules/menuform.py",
           getenv("GISBASE"));
-    G_spawn(getenv("GRASS_PYTHON"), "menuform.py", script, pgm_name, NULL);
+    G_spawn(getenv("GRASS_PYTHON"), getenv("GRASS_PYTHON"), script, pgm_name, NULL);
 }

where I am unsure. I assume that this is also needed.

comment:17 in reply to:  16 ; Changed 10 years ago by glynn

Replying to neteler:

I assume that this is also needed.

Only for Gentoo, AFAIK, but it won't harm elsewhere (most programs don't care about argv[0]).

If you run a script via the shell (manually or with system()), argv[0] is set to the interpreter (normally to the exact path following the #!, although in the case of the "#!/usr/bin/env python" trick, argv[0] will be just "python").

So the updated version simulates "normal" execution.

comment:18 in reply to:  17 Changed 10 years ago by neteler

Resolution: fixed
Status: reopenedclosed

Replying to glynn:

Replying to neteler:

I assume that this is also needed.

Only for Gentoo, AFAIK, but it won't harm elsewhere (most programs don't care about argv[0]).

Fixed in r40722, also to make parser.c more similar to 6.5. Closing again as all seems to be ok now.

comment:19 in reply to:  17 Changed 10 years ago by marisn

Replying to glynn:

Only for Gentoo, AFAIK, but it won't harm elsewhere (most programs don't care about argv[0]).

Just for record - this Gentoo issue will be fixed at some point. More info in Gentoo bugzilla http://bugs.gentoo.org/show_bug.cgi?id=286191

Note: See TracTickets for help on using tickets.