[GRASS5] Re: [GRASS-CVS] glynn: grass51/general/g.parser main.c,2.0,2.1

GRASS GIS GRASS GIS developer
1

Hi,

does the change below affect existing GRASS scripts?

Markus

On Tue, Nov 30, 2004 at 12:16:45AM +0100, grass@intevation.de wrote:

Author: glynn

Update of /grassrepository/grass51/general/g.parser
In directory doto:/tmp/cvs-serv8330/general/g.parser

Modified Files:
  main.c
Log Message:
Unused options should be empty rather than "(null)".

Index: main.c

RCS file: /grassrepository/grass51/general/g.parser/main.c,v
retrieving revision 2.0
retrieving revision 2.1
diff -u -d -r2.0 -r2.1
--- main.c 9 Nov 2004 14:08:27 -0000 2.0
+++ main.c 29 Nov 2004 23:16:43 -0000 2.1
@@ -290,7 +290,7 @@
     for (option = ctx.first_option; option; option = option->next_opt)
     {
   char buff[1024];
- sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer);
+ sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer ? option->answer : "");
   putenv(G_store(buff));
     }

_______________________________________________
grass-commit mailing list
grass-commit@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass-commit

2

Markus Neteler wrote:

does the change below affect existing GRASS scripts?

> - sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer);
> + sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer ? option->answer : "");

The only way in which it could affect existing scripts is if they are
doing e.g.:

  if [ $GIS_OPT_something = '(null)' ] ; then
    ...

which is probably a bug. I'm not sure that sprintf(buff, "%s", NULL)
is guaranteed to generate "(null)".

However, there is at least one script (i.image.mosaic) which expects a
missing argument to result in $GIS_OPT_* being the empty string.

N.B.: this is a fix for bug #2740.

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

3

> does the change below affect existing GRASS scripts?

> > - sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer);
> > + sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer ? option->answer : "");

The only way in which it could affect existing scripts is if they are
doing e.g.:

  if [ $GIS_OPT_something = '(null)' ] ; then
    ...

which is probably a bug. I'm not sure that sprintf(buff, "%s", NULL)
is guaranteed to generate "(null)".

However, there is at least one script (i.image.mosaic) which expects a
missing argument to result in $GIS_OPT_* being the empty string.

N.B.: this is a fix for bug #2740.

grass57/scripts$ grep -r '(null)' * | cut -f1 -d/ | uniq
d.monsize
d.out.png
d.rast.leg
d.slide.show
g.manual
g.mlist
g.mremove
i.spectral
r.out.gdal
r.reclass.area
r.shaded.relief
v.in.garmin

Is the above change permanent and should all these scripts change to

- if [ $GIS_OPT_something = '(null)' ] ; then
+ if [ -z $GIS_OPT_something ] ; then
   ...

either way is ok, but we need to pick one way and stick with it.

'db.connect -p' also shows (null) from a new mapset.

?
Hamish

4

On Sun, Dec 05, 2004 at 04:22:04PM +1300, Hamish wrote:

> > does the change below affect existing GRASS scripts?
>
> > > - sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer);
> > > + sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer ? option->answer : "");
>
> The only way in which it could affect existing scripts is if they are
> doing e.g.:
>
> if [ $GIS_OPT_something = '(null)' ] ; then
> ...
>
> which is probably a bug. I'm not sure that sprintf(buff, "%s", NULL)
> is guaranteed to generate "(null)".
>
> However, there is at least one script (i.image.mosaic) which expects a
> missing argument to result in $GIS_OPT_* being the empty string.
>
> N.B.: this is a fix for bug #2740.

grass57/scripts$ grep -r '(null)' * | cut -f1 -d/ | uniq
d.monsize
d.out.png
d.rast.leg
d.slide.show
g.manual
g.mlist
g.mremove
i.spectral
r.out.gdal
r.reclass.area
r.shaded.relief
v.in.garmin

Is the above change permanent and should all these scripts change to

- if [ $GIS_OPT_something = '(null)' ] ; then
+ if [ -z $GIS_OPT_something ] ; then
   ...

either way is ok, but we need to pick one way and stick with it.

'db.connect -p' also shows (null) from a new mapset.

I think that this has to be urgently resolved.
Glynn, please suggest a solution...

Markus

5

Hamish wrote:

> > does the change below affect existing GRASS scripts?
>
> > > - sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer);
> > > + sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer ? option->answer : "");
>
> The only way in which it could affect existing scripts is if they are
> doing e.g.:
>
> if [ $GIS_OPT_something = '(null)' ] ; then
> ...
>
> which is probably a bug. I'm not sure that sprintf(buff, "%s", NULL)
> is guaranteed to generate "(null)".
>
> However, there is at least one script (i.image.mosaic) which expects a
> missing argument to result in $GIS_OPT_* being the empty string.
>
> N.B.: this is a fix for bug #2740.

grass57/scripts$ grep -r '(null)' * | cut -f1 -d/ | uniq
d.monsize
d.out.png
d.rast.leg
d.slide.show
g.manual
g.mlist
g.mremove
i.spectral
r.out.gdal
r.reclass.area
r.shaded.relief
v.in.garmin

Is the above change permanent and should all these scripts change to

- if [ $GIS_OPT_something = '(null)' ] ; then
+ if [ -z $GIS_OPT_something ] ; then
   ...

Yes.

either way is ok, but we need to pick one way and stick with it.

Using (null) assumes that printf("%s") will represent null pointers
that way (rather than just e.g. segfaulting).

The fix has the disadvantage that you can't distinguish between an
omitted option and one which was explicitly set to the empty string. I
don't know if that really matters.

If it's essential to be able to distinguish these two cases, there
will need to be an extra variable for each option, e.g.
$GIS_OPT_USED_something.

[With the original, you can't distinguish between an omitted option
and one which was explicitly set to "(null)", but that's even less
likely to matter.]

'db.connect -p' also shows (null) from a new mapset.

Ideally that should be fixed, e.g.

- printf("... %s ...", ptr)
+ printf("... %s ...", ptr ? ptr : "(none)");

According to ANSI C, the effect of passing a null pointer for %s is
undefined, so we shouldn't do it.

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

6

I can help to change these scripts if there is a decision as to what needs
to be done.

Michael

On 12/6/04 3:57 AM, "Markus Neteler" <neteler@itc.it> wrote:

On Sun, Dec 05, 2004 at 04:22:04PM +1300, Hamish wrote:

does the change below affect existing GRASS scripts?

- sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer);
+ sprintf(buff, "GIS_OPT_%s=%s", option->key, option->answer ?
option->answer : "");

The only way in which it could affect existing scripts is if they are
doing e.g.:

if [ $GIS_OPT_something = '(null)' ] ; then
...

which is probably a bug. I'm not sure that sprintf(buff, "%s", NULL)
is guaranteed to generate "(null)".

However, there is at least one script (i.image.mosaic) which expects a
missing argument to result in $GIS_OPT_* being the empty string.

N.B.: this is a fix for bug #2740.

grass57/scripts$ grep -r '(null)' * | cut -f1 -d/ | uniq
d.monsize
d.out.png
d.rast.leg
d.slide.show
g.manual
g.mlist
g.mremove
i.spectral
r.out.gdal
r.reclass.area
r.shaded.relief
v.in.garmin

Is the above change permanent and should all these scripts change to

- if [ $GIS_OPT_something = '(null)' ] ; then
+ if [ -z $GIS_OPT_something ] ; then
   ...

either way is ok, but we need to pick one way and stick with it.

'db.connect -p' also shows (null) from a new mapset.

I think that this has to be urgently resolved.
Glynn, please suggest a solution...

Markus

______________________________
Michael Barton, Professor of Anthropology
School of Human, Evolution and Social Change
Arizona State University
Tempe, AZ 85287-2402
USA

voice: 480-965-6262; fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton