[GRASS-dev] MASK_OVERRIDE

[Ivan_Shmakov1]

  Since it's somewhat cumbersome to work with multiple `MASK's
  currently (`r.reclass', `g.copy'?), I'd like to introduce
  `MASK_OVERRIDE' -- an environment variable analogous to
  `WIND_OVERRIDE'.

lib/gis/auto_mask.c (G__check_for_auto_masking): Handle `MASK_OVERRIDE';
allow mask to be in any mapset in the search path.
lib/gis/mask_info.c (G__mask_info): Likewise.

diff --git a/lib/gis/auto_mask.c b/lib/gis/auto_mask.c
index 6828018..0dc218d 100644
--- a/lib/gis/auto_mask.c
+++ b/lib/gis/auto_mask.c
@@ -32,6 +32,7 @@

int G__check_for_auto_masking (void)
{
+ char name[GNAME_MAX] = "MASK", mapset[GMAPSET_MAX] = "";
     struct Cell_head cellhd;

     /* if mask is switched off (-2) return -2
@@ -42,14 +43,26 @@ int G__check_for_auto_masking (void)

     /* if(G__.mask_fd > 0) G_free (G__.mask_buf);*/

- /* look for the existence of the MASK file */
- G__.auto_mask = (G_find_cell ("MASK", G_mapset()) != 0);
+ /* check for MASK_OVERRIDE */
+ {
+ const char *override = getenv ("MASK_OVERRIDE");
+ if (override != 0) strncpy (name, override, sizeof (name));
+ }
+
+ /* look for the existence of the MASK file; get the mapset name */
+ {
+ char *mask_mapset = G_find_cell (name, mapset);
+
+ if ((G__.auto_mask = (mask_mapset != 0)) > 0) {
+ strncpy (mapset, mask_mapset, sizeof (mapset));
+ }
+ }

     if (G__.auto_mask <= 0)
         return 0;

     /* check MASK projection/zone against current region */
- if (G_get_cellhd ("MASK", G_mapset(), &cellhd) >= 0)
+ if (G_get_cellhd (name, mapset, &cellhd) >= 0)
     {
   if (cellhd.zone != G_zone() || cellhd.proj != G_projection())
   {
@@ -59,11 +72,12 @@ int G__check_for_auto_masking (void)
     }

     G_unopen_cell(G__.mask_fd );
- G__.mask_fd = G__open_cell_old ("MASK", G_mapset());
+ G__.mask_fd = G__open_cell_old (name, mapset);
     if (G__.mask_fd < 0)
     {
         G__.auto_mask = 0;
- G_warning (_("Unable to open automatic MASK file"));
+ G_warning (_("Unable to open automatic MASK (<%s@%s>) file"),
+ name, mapset);
         return 0;
     }

diff --git a/lib/gis/mask_info.c b/lib/gis/mask_info.c
index b82e8ea..dd7f905 100644
--- a/lib/gis/mask_info.c
+++ b/lib/gis/mask_info.c
@@ -58,11 +58,32 @@ int G__mask_info (
{
     char rname[GNAME_MAX], rmapset[GMAPSET_MAX];

- strcpy (name, "MASK");
- strcpy (mapset, G_mapset());
+ /* NB: the following is painfully close to G__check_for_auto_masking
+ * (); consider a separate function here
+ */

- if(!G_find_cell (name, mapset))
- return -1;
+ /* check for MASK_OVERRIDE */
+ {
+ const char *override = getenv ("MASK_OVERRIDE");
+ if (override != 0) {
+ strncpy (name, override, sizeof (name));
+ } else {
+ strcpy (name, "MASK");
+ }
+ }
+
+ /* look for the existence of the MASK file; get the mapset name */
+ {
+ char *mask_mapset = G_find_cell (name, "");
+
+ if (mask_mapset == 0) {
+ /* NB: `mapset' won't be initialized */
+ /* . */
+ return -1;
+ }
+
+ strncpy (mapset, mask_mapset, sizeof (mapset));
+ }

     if(G_is_reclass (name, mapset, rname, rmapset) > 0)
     {

[Glynn_Clements1]

Ivan Shmakov wrote:

  Since it's somewhat cumbersome to work with multiple `MASK's
  currently (`r.reclass', `g.copy'?),

Once you have a set of maps suitable for use as masks, switching
between them just requires using g.rename or "g.copy --o". The
r.reclass (or r.mapcalc) step still needs to be done regardless.

I'd like to introduce
  `MASK_OVERRIDE' -- an environment variable analogous to
  `WIND_OVERRIDE'.

This would eliminate potential problems with running commands
concurrently. E.g. if you set a mask from the command line, any
display commands used by the GUI will also use the mask.

So, this is really more an issue of flexibility than convenience. It's
probably easier for the user to just use g.rename/g.copy.

lib/gis/auto_mask.c (G__check_for_auto_masking): Handle `MASK_OVERRIDE';
allow mask to be in any mapset in the search path.
lib/gis/mask_info.c (G__mask_info): Likewise.

diff --git a/lib/gis/auto_mask.c b/lib/gis/auto_mask.c
index 6828018..0dc218d 100644
--- a/lib/gis/auto_mask.c
+++ b/lib/gis/auto_mask.c
@@ -32,6 +32,7 @@

int G__check_for_auto_masking (void)
{
+ char name[GNAME_MAX] = "MASK", mapset[GMAPSET_MAX] = "";
     struct Cell_head cellhd;

     /* if mask is switched off (-2) return -2
@@ -42,14 +43,26 @@ int G__check_for_auto_masking (void)

     /* if(G__.mask_fd > 0) G_free (G__.mask_buf);*/

- /* look for the existence of the MASK file */
- G__.auto_mask = (G_find_cell ("MASK", G_mapset()) != 0);
+ /* check for MASK_OVERRIDE */
+ {
+ const char *override = getenv ("MASK_OVERRIDE");
+ if (override != 0) strncpy (name, override, sizeof (name));

Use "if (override)" rather than an explicit comparison against 0.
Although C allows comparisons between pointers and literal 0, you may
get a warning, and we have enough of those cluttering up the build
output already. Also, NULL/non-NULL pointers are conventionally as
much of a logic value as zero/non-zero integers. Presumably you
wouldn't write "if ((x == y) != 0)" instead of "if (x == y)"?

+ }
+
+ /* look for the existence of the MASK file; get the mapset name */
+ {
+ char *mask_mapset = G_find_cell (name, mapset);
+
+ if ((G__.auto_mask = (mask_mapset != 0)) > 0) {
+ strncpy (mapset, mask_mapset, sizeof (mapset));
+ }
+ }

For this feature to be of real use, you need to allow MASK_OVERRIDE to
be used to disable reading a mask altogether. E.g. if MASK_OVERRIDE is
defined but empty, no mask should be used rather than falling back to
MASK. This would allow the GUI to operate without a mask even when
MASK has been created via the command line.

diff --git a/lib/gis/mask_info.c b/lib/gis/mask_info.c
index b82e8ea..dd7f905 100644
--- a/lib/gis/mask_info.c
+++ b/lib/gis/mask_info.c
@@ -58,11 +58,32 @@ int G__mask_info (

     if(G_is_reclass (name, mapset, rname, rmapset) > 0)
     {

Personally, I think that returning the base map in the case of a
reclass map is probably bogus. There may be some point to it when the
actual mask will always be MASK@<current mapset>, but if you can
configure it with a variable, the actual mask map will be more useful
than the base map.

I suggest changing G_mask_info() to print the name of the actual mask
map (i.e. either $MASK_OVERRIDE or "MASK@mapset"), as well as the base
map if it's a reclass map.

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

[Ivan_Shmakov1]

Glynn Clements <glynn@gclements.plus.com> writes:

>> Since it's somewhat cumbersome to work with multiple `MASK's
>> currently (`r.reclass', `g.copy'?),

> Once you have a set of maps suitable for use as masks, switching
> between them just requires using g.rename or "g.copy --o".

  Yes. Though, in my opinion, it's hardly as convenient as,
  e. g.:

$ MASK_OVERRIDE=my_mask r.stats -c my_raster

$ MASK_OVERRIDE=my_mask bash process-it-all-for-me.sh

> The r.reclass (or r.mapcalc) step still needs to be done regardless.

  The calculation of the mask needs to be done just once. The
  point was that one may use an ``identity'' `r.reclass' to create
  `MASK' as an alias to some existing raster.

>> I'd like to introduce `MASK_OVERRIDE' -- an environment variable
>> analogous to `WIND_OVERRIDE'.

> This would eliminate potential problems with running commands
> concurrently. E.g. if you set a mask from the command line, any
> display commands used by the GUI will also use the mask.

  Yes.

> So, this is really more an issue of flexibility than
> convenience. It's probably easier for the user to just use
> g.rename/g.copy.

  Not for me, at least.

[...]

>> + {
>> + const char *override = getenv ("MASK_OVERRIDE");
>> + if (override != 0) strncpy (name, override, sizeof (name));

> Use "if (override)" rather than an explicit comparison against 0.
> Although C allows comparisons between pointers and literal 0, you may
> get a warning, and we have enough of those cluttering up the build
> output already. Also, NULL/non-NULL pointers are conventionally as
> much of a logic value as zero/non-zero integers.

  ... The thing I cannot get used in C...

> Presumably you wouldn't write "if ((x == y) != 0)" instead of "if (x
> == y)"?

[...]

> For this feature to be of real use, you need to allow MASK_OVERRIDE
> to be used to disable reading a mask altogether. E.g. if
> MASK_OVERRIDE is defined but empty, no mask should be used rather
> than falling back to MASK. This would allow the GUI to operate
> without a mask even when MASK has been created via the command line.

  The code doesn't ever fall back to `MASK' if `MASK_OVERRIDE' is
  defined. If `MASK_OVERRIDE' doesn't point to a valid raster
  (e. g., if it's an empty string), the mask is ignored.

  However, I doubt if it's a reasonable behaviour. It seems to be
  usual in the Unix world to ignore the environment variable's
  value if it's an empty string. Using a non-existing raster
  (say, `.') is probably a better idea.

[...]

>> if(G_is_reclass (name, mapset, rname, rmapset) > 0) {

> Personally, I think that returning the base map in the case of a
> reclass map is probably bogus. There may be some point to it when the
> actual mask will always be MASK@<current mapset>, but if you can
> configure it with a variable, the actual mask map will be more useful
> than the base map.

> I suggest changing G_mask_info() to print the name of the actual mask
> map (i.e. either $MASK_OVERRIDE or "MASK@mapset"), as well as the
> base map if it's a reclass map.

  I agree, but this deserves a separate entry in the SVN log.

  I'll post a revised diff later.

[Ivan_Shmakov1]

  Here's a revised patch.

lib/gis/auto_mask.c (G__check_for_auto_masking): Handle `MASK_OVERRIDE';
allow mask to be in any mapset in the search path.
lib/gis/mask_info.c (G__mask_info): Likewise.

  I believe I've fixed all the `strncpy' issues that my previous
  patch has had. Also, I've changed the interpretation of the
  empty string value for `MASK_OVERRIDE' to ``ignore the
  variable's value'', since I believe it matches the rest of the
  Unix world. (One still can ask for ``no mask'' by specifying a
  non-existent raster via `MASK_OVERRIDE'.)

  Since there're some modules accessing `MASK', there should be a
  libgis function to obtain the name of the mask currently in use
  (besides, this would simplify the code below.) Any suggestions?

diff --git a/lib/gis/auto_mask.c b/lib/gis/auto_mask.c
index 6828018..ada9adb 100644
--- a/lib/gis/auto_mask.c
+++ b/lib/gis/auto_mask.c
@@ -32,6 +32,7 @@

int G__check_for_auto_masking (void)
{
+ char name[GNAME_MAX] = "MASK", mapset[GMAPSET_MAX] = "";
     struct Cell_head cellhd;

     /* if mask is switched off (-2) return -2
@@ -42,14 +43,32 @@ int G__check_for_auto_masking (void)

     /* if(G__.mask_fd > 0) G_free (G__.mask_buf);*/

- /* look for the existence of the MASK file */
- G__.auto_mask = (G_find_cell ("MASK", G_mapset()) != 0);
+ /* check for MASK_OVERRIDE */
+ {
+ const char *override = getenv ("MASK_OVERRIDE");
+
+ /* an empty string value makes the variable to be ignored */
+ if (override && override[0] != '\0') {
+ strncpy (name, override, sizeof (name) - 1);
+ name[sizeof (name) - 1] = '\0';
+ }
+ }
+
+ /* look for the existence of the MASK file; get the mapset name */
+ {
+ char *mask_mapset = G_find_cell (name, mapset);
+
+ if ((G__.auto_mask = (mask_mapset != 0)) > 0) {
+ strncpy (mapset, mask_mapset, sizeof (mapset) - 1);
+ mapset[sizeof (mapset) - 1] = '\0';
+ }
+ }

     if (G__.auto_mask <= 0)
         return 0;

     /* check MASK projection/zone against current region */
- if (G_get_cellhd ("MASK", G_mapset(), &cellhd) >= 0)
+ if (G_get_cellhd (name, mapset, &cellhd) >= 0)
     {
   if (cellhd.zone != G_zone() || cellhd.proj != G_projection())
   {
@@ -59,11 +78,12 @@ int G__check_for_auto_masking (void)
     }

     G_unopen_cell(G__.mask_fd );
- G__.mask_fd = G__open_cell_old ("MASK", G_mapset());
+ G__.mask_fd = G__open_cell_old (name, mapset);
     if (G__.mask_fd < 0)
     {
         G__.auto_mask = 0;
- G_warning (_("Unable to open automatic MASK file"));
+ G_warning (_("Unable to open automatic MASK (<%s@%s>) file"),
+ name, mapset);
         return 0;
     }

diff --git a/lib/gis/mask_info.c b/lib/gis/mask_info.c
index b82e8ea..3687ab1 100644
--- a/lib/gis/mask_info.c
+++ b/lib/gis/mask_info.c
@@ -58,11 +58,34 @@ int G__mask_info (
{
     char rname[GNAME_MAX], rmapset[GMAPSET_MAX];

- strcpy (name, "MASK");
- strcpy (mapset, G_mapset());
+ /* NB: the following is painfully close to G__check_for_auto_masking
+ * (); consider a separate function here
+ */

- if(!G_find_cell (name, mapset))
- return -1;
+ /* check for MASK_OVERRIDE */
+ {
+ const char *override = getenv ("MASK_OVERRIDE");
+ if (override != 0) {
+ strncpy (name, override, GNAME_MAX - 1);
+ name[GNAME_MAX - 1] = '\0';
+ } else {
+ strcpy (name, "MASK");
+ }
+ }
+
+ /* look for the existence of the MASK file; get the mapset name */
+ {
+ char *mask_mapset = G_find_cell (name, "");
+
+ if (mask_mapset == 0) {
+ /* NB: `mapset' won't be initialized */
+ /* . */
+ return -1;
+ }
+
+ strncpy (mapset, mask_mapset, GMAPSET_MAX - 1);
+ mapset[GMAPSET_MAX - 1] = '\0';
+ }

     if(G_is_reclass (name, mapset, rname, rmapset) > 0)
     {

[Paul_Kelly]

On Wed, 20 Feb 2008, Ivan Shmakov wrote:

Glynn Clements <glynn@gclements.plus.com> writes:

> For this feature to be of real use, you need to allow MASK_OVERRIDE
> to be used to disable reading a mask altogether. E.g. if
> MASK_OVERRIDE is defined but empty, no mask should be used rather
> than falling back to MASK. This would allow the GUI to operate
> without a mask even when MASK has been created via the command line.

  The code doesn't ever fall back to `MASK' if `MASK_OVERRIDE' is
  defined. If `MASK_OVERRIDE' doesn't point to a valid raster
  (e. g., if it's an empty string), the mask is ignored.

  However, I doubt if it's a reasonable behaviour. It seems to be
  usual in the Unix world to ignore the environment variable's
  value if it's an empty string. Using a non-existing raster
  (say, `.') is probably a better idea.

On Windows, an environment variable is unset by setting it to an empty string. There seems to be no difference between the two. So I don't think we could rely on the behaviour caused by MASK_OVERRIDE being an empty string. OTOH setting it to some garbage value seems a little unelegant, but as long as it's not a valid raster name there should be no problem I suppose.

Paul

[Ivan_Shmakov1]

Paul Kelly <paul-grass@stjohnspoint.co.uk> writes:

>>> For this feature to be of real use, you need to allow MASK_OVERRIDE
>>> to be used to disable reading a mask altogether. E.g. if
>>> MASK_OVERRIDE is defined but empty, no mask should be used rather
>>> than falling back to MASK. This would allow the GUI to operate
>>> without a mask even when MASK has been created via the command
>>> line.

[...]

>> It seems to be usual in the Unix world to ignore the environment
>> variable's value if it's an empty string. Using a non-existing
>> raster (say, `.') is probably a better idea.

> On Windows, an environment variable is unset by setting it to an
> empty string. There seems to be no difference between the two. So I
> don't think we could rely on the behaviour caused by MASK_OVERRIDE
> being an empty string.

  If it's so, there should probably a GRASS-wide policy on
  ignoring environment variables set to an empty string. Why not
  a libgis function?

const char *
getenv_nonempty (const char *name)
{
  const char *value = getenv (name);
  return (value && *value != '\0') ? value : 0;
}

> OTOH setting it to some garbage value seems a little unelegant, but
> as long as it's not a valid raster name there should be no problem I
> suppose.

  I'm still open to suggestions.

  If none, I'd probably add a `.'-check to the code in order to
  avoid a bogus warning.

[Glynn_Clements1]

Ivan Shmakov wrote:

>> Since it's somewhat cumbersome to work with multiple `MASK's
>> currently (`r.reclass', `g.copy'?),

> Once you have a set of maps suitable for use as masks, switching
> between them just requires using g.rename or "g.copy --o".

  Yes. Though, in my opinion, it's hardly as convenient as,
  e. g.:

$ MASK_OVERRIDE=my_mask r.stats -c my_raster

$ MASK_OVERRIDE=my_mask bash process-it-all-for-me.sh

An environment variable is probably easier if you're only using the
mask for a single command, provided that the user has a working
knowledge of their shell (you'd be surprised how many don't; or maybe
you wouldn't).

If you're using a mask for multiple commands, g.copy is probably
easier. It doesn't require knowledge of shell variables versus
environment variables, and it can be wrapped inside a script (whereas
you can't make a script "export" to the shell from which it's called).

>> + {
>> + const char *override = getenv ("MASK_OVERRIDE");
>> + if (override != 0) strncpy (name, override, sizeof (name));

> Use "if (override)" rather than an explicit comparison against 0.
> Although C allows comparisons between pointers and literal 0, you may
> get a warning, and we have enough of those cluttering up the build
> output already. Also, NULL/non-NULL pointers are conventionally as
> much of a logic value as zero/non-zero integers.

  ... The thing I cannot get used in C...

What thing? That C doesn't have a distinct boolean type?

> For this feature to be of real use, you need to allow MASK_OVERRIDE
> to be used to disable reading a mask altogether. E.g. if
> MASK_OVERRIDE is defined but empty, no mask should be used rather
> than falling back to MASK. This would allow the GUI to operate
> without a mask even when MASK has been created via the command line.

  The code doesn't ever fall back to `MASK' if `MASK_OVERRIDE' is
  defined. If `MASK_OVERRIDE' doesn't point to a valid raster
  (e. g., if it's an empty string), the mask is ignored.

Okay; I misread the part where it was saving the mapset.

AFAICT, it shouldn't be necessary to handle the mapset explicitly. The
lower level functions should all accept a qualified name, i.e.:

  G__open_cell_old("mask@mapset", "")

So you should be able to simply test for existence with
G_find_cell2(), and store the (possibly qualified) name.

Ideally, I'd like to remove the mapset argument from the core libgis
functions in 7.x. It will involve modifying almost every single
module, but it will make writing new modules and maintaining existing
modules simpler.

The only code which really needs to split a qualified name is in low
level functions which actually open files, plus anything which creates
a derived name (i.e. foo@bar -> foo.tmp@bar not foo@bar.tmp).

  However, I doubt if it's a reasonable behaviour. It seems to be
  usual in the Unix world to ignore the environment variable's
  value if it's an empty string. Using a non-existing raster
  (say, `.') is probably a better idea.

Good point. It's a nuisance for a shell script to distinguish between
a variable being unset and being set to the empty string. We should
really choose a specific "none" value rather than just silently
ignoring a reference to a non-existent map. If $MASK_OVERRIDE is set
but the map doesn't exist, it should at least produce a warning.

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

[Ivan_Shmakov1]

Glynn Clements <glynn@gclements.plus.com> writes:

[...]

>>> build output already. Also, NULL/non-NULL pointers are
>>> conventionally as much of a logic value as zero/non-zero integers.

>> ... The thing I cannot get used in C...

> What thing? That C doesn't have a distinct boolean type?

  That C allows a pointer anywhere an integer is allowed.

  I always read an `anytype *' variable as a ``reference to
  object'', or even simply ``object''. It thus somewhat strange
  to read ``if reference, then...''

[...]

>> The code doesn't ever fall back to `MASK' if `MASK_OVERRIDE' is
>> defined. If `MASK_OVERRIDE' doesn't point to a valid raster (e. g.,
>> if it's an empty string), the mask is ignored.

> Okay; I misread the part where it was saving the mapset.

> AFAICT, it shouldn't be necessary to handle the mapset
> explicitly. The lower level functions should all accept a qualified
> name, i.e.:

> G__open_cell_old("mask@mapset", "")

> So you should be able to simply test for existence with
> G_find_cell2(), and store the (possibly qualified) name.

  I've tried to trace the calls made by the functions I've
  modified, but haven't found enough evidence that this usage
  pattern is allowed. Could you please confirm that every
  function to be called with the `MASK_OVERRIDE's value in the
  modified code can be used this way?

> Ideally, I'd like to remove the mapset argument from the core libgis
> functions in 7.x. It will involve modifying almost every single
> module, but it will make writing new modules and maintaining existing
> modules simpler.

  I'm in favour of such a change.

> The only code which really needs to split a qualified name is in low
> level functions which actually open files, plus anything which
> creates a derived name (i.e. foo@bar -> foo.tmp@bar not foo@bar.tmp).

  What are the functions to:

  * for any valid (possibly qualified) reference for an object
    (raster, vector, etc.) existing in one of the mapsets
    comprising the mapset search path, return a corresponding
    qualified object name;

  * like the above, but returning the name and the mapset
    separately?

  Since the code of both the functions I've modified looks
  painfully similar, may I ask for a function to return the
  (qualified?) name of the (existing?) mask? Which file should I
  put it in?

  (There're some modules which could use this function as well.)

>> However, I doubt if it's a reasonable behaviour. It seems to be
>> usual in the Unix world to ignore the environment variable's value
>> if it's an empty string. Using a non-existing raster (say, `.') is
>> probably a better idea.

> Good point. It's a nuisance for a shell script to distinguish between
> a variable being unset and being set to the empty string. We should
> really choose a specific "none" value rather than just silently
> ignoring a reference to a non-existent map. If $MASK_OVERRIDE is set
> but the map doesn't exist, it should at least produce a warning.

  For now I'm considering using any name starting with `.' to
  specify ``none''. The documentation should probably state that
  `MASK_OVERRIDE=.' is the preferred way to off the mask.

  Unless there're other suggestions, I'd code it to check for
  `name[0] == '.'' and return early with a ``no mask found''
  condition.

[Glynn_Clements1]

Ivan Shmakov wrote:

>>> build output already. Also, NULL/non-NULL pointers are
>>> conventionally as much of a logic value as zero/non-zero integers.

>> ... The thing I cannot get used in C...

> What thing? That C doesn't have a distinct boolean type?

  That C allows a pointer anywhere an integer is allowed.

  I always read an `anytype *' variable as a ``reference to
  object'', or even simply ``object''. It thus somewhat strange
  to read ``if reference, then...''

In many contexts, using a pointer where an integer is expected will
produce at least a warning (e.g. performing multiplication on a
pointer). Using a pointer as the test of an if, while or for statement
is safe as it only cares about zero or non-zero.

Technically:

       [#21] Integer and floating types are collectively called
       arithmetic types. Arithmetic types and pointer types are
       collectively called scalar types. Array and structure types
       are collectively called aggregate types.32)

The "controlling expression" of an if, while or for statement must
have scalar type (i.e. integer, floating or pointer type), as must the
operands to the &&, || and ! operators and the first operand of the ?:
operator.

>> The code doesn't ever fall back to `MASK' if `MASK_OVERRIDE' is
>> defined. If `MASK_OVERRIDE' doesn't point to a valid raster (e. g.,
>> if it's an empty string), the mask is ignored.

> Okay; I misread the part where it was saving the mapset.

> AFAICT, it shouldn't be necessary to handle the mapset
> explicitly. The lower level functions should all accept a qualified
> name, i.e.:

> G__open_cell_old("mask@mapset", "")

> So you should be able to simply test for existence with
> G_find_cell2(), and store the (possibly qualified) name.

  I've tried to trace the calls made by the functions I've
  modified, but haven't found enough evidence that this usage
  pattern is allowed. Could you please confirm that every
  function to be called with the `MASK_OVERRIDE's value in the
  modified code can be used this way?

Some time ago, I went through libgis and changed everything I could
find to accept qualified names. Most functions just pass the name and
mapset arguments down the chain until they get to G__open() or
G__file_name(), which definitely accept qualified names.

It's conceivable that I could have missed some function in the middle
which internally depends upon a name being unqualified. But if core
functions such as G_get_cellhd() or G__open_cell_old() didn't handle
it, I think that we would have noticed by now.

> The only code which really needs to split a qualified name is in low
> level functions which actually open files, plus anything which
> creates a derived name (i.e. foo@bar -> foo.tmp@bar not foo@bar.tmp).

  What are the functions to:

  * for any valid (possibly qualified) reference for an object
    (raster, vector, etc.) existing in one of the mapsets
    comprising the mapset search path, return a corresponding
    qualified object name;

I don't think such a function exists at present. It would be easy
enough to add if there was any use for it; it's just a combination of
G_find_{cell,vector,<etc>} and G_fully_qualified_name().

  * like the above, but returning the name and the mapset
    separately?

G_find_{cell,vector,<etc>}.

  Since the code of both the functions I've modified looks
  painfully similar, may I ask for a function to return the
  (qualified?) name of the (existing?) mask? Which file should I
  put it in?

  (There're some modules which could use this function as well.)

I would say find_file.c for a generic version, with type-specific
versions in find_cell.c and find_vect.c. The functions in nme_in_mps.c
are related, but they only do string operations, not I/O.

>> However, I doubt if it's a reasonable behaviour. It seems to be
>> usual in the Unix world to ignore the environment variable's value
>> if it's an empty string. Using a non-existing raster (say, `.') is
>> probably a better idea.

> Good point. It's a nuisance for a shell script to distinguish between
> a variable being unset and being set to the empty string. We should
> really choose a specific "none" value rather than just silently
> ignoring a reference to a non-existent map. If $MASK_OVERRIDE is set
> but the map doesn't exist, it should at least produce a warning.

  For now I'm considering using any name starting with `.' to
  specify ``none''. The documentation should probably state that
  `MASK_OVERRIDE=.' is the preferred way to off the mask.

  Unless there're other suggestions, I'd code it to check for
  `name[0] == '.'' and return early with a ``no mask found''
  condition.

Agreed.

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

[Ivan_Shmakov1]

Glynn Clements <glynn@gclements.plus.com> writes:

[...]

>> Since the code of both the functions I've modified looks painfully
>> similar, may I ask for a function to return the (qualified?) name of
>> the (existing?) mask? Which file should I put it in?

>> (There're some modules which could use this function as well.)

> I would say find_file.c for a generic version, with type-specific
> versions in find_cell.c and find_vect.c.

  ? Are there a ``generic'' mask facility?

> The functions in nme_in_mps.c are related, but they only do string
> operations, not I/O.

[...]

[Glynn_Clements1]

Ivan Shmakov wrote:

>> Since the code of both the functions I've modified looks painfully
>> similar, may I ask for a function to return the (qualified?) name of
>> the (existing?) mask? Which file should I put it in?

>> (There're some modules which could use this function as well.)

> I would say find_file.c for a generic version, with type-specific
> versions in find_cell.c and find_vect.c.

  ? Are there a ``generic'' mask facility?

No; I was thinking mainly in terms of this part:

> What are the functions to:
>
> * for any valid (possibly qualified) reference for an object
> (raster, vector, etc.) existing in one of the mapsets
> comprising the mapset search path, return a corresponding
> qualified object name;

I don't think such a function exists at present. It would be easy
enough to add if there was any use for it; it's just a combination of
G_find_{cell,vector,<etc>} and G_fully_qualified_name().

"Finding" a map (or other "element") in a mapset has both generic
versions (G_find_file()) and type-specific versions (G_find_cell(),
G_find_vector()), so a G_find_* variant which returns the qualified
name would have generic and type-specific versions.

A function solely to return the qualified name of the current mask
should probably go into mask_info.c.

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