[GRASS-dev] Use of "const", qualified map names

[Glynn Clements]

Glynn Clements wrote:

> Over the last two days I have gone through libgis and added "const"
> qualifiers wherever they were appropriate (as well as a few other
> minor clean-ups).
> 
> However, this required a subtle API change.
> 
> Previously, just about every libgis function which accepted parameters
> of the form (char *name, char *mapset, ...) would end up normalising
> the name (removing any "@mapset" part). In most cases, this was likely
> to be accidental: an artifact of using G_find_file() etc.
> 
> Apart from meaning that you couldn't pass a "const char *" for the
> name, it could cause problems if you weren't expecting it (certainly,
> I've been bitten by it at least once).
> 
> Following my changes, the only functions which should strip the
> @mapset part from a map name are the G_find_{file,cell,vector}
> functions (which return the mapset where the file was found). Anything
> else should leave the name intact (but will still honour any @mapset
> part).
> 
> There is a remote possibility that some module somewhere was actually
> relying upon the existing behaviour (rather than using G_find_*() to
> explicitly normalise the name). As this would be almost impossible to
> track down methodically, all that I can suggest is to be on the
> lookout for modules misbehaving when given qualified map names.

Soeren found one major instance: G__check_fp_type(), used by
G__open_cell_old() (and thus everything which uses it).

This caused a significant number of modules to fail when reading FP
maps.

I've fixed that particular case in CVS, but there are others. Mostly,
they relate to code which accesses the cell_misc directory.

[Aside: whoever came up with the cell_misc structure needs major
therapy. Getting rid of it needs to be the #1 priority for 7.x.]

The reason is that all of the G_open_* functions which take
element/name (and sometimes mapset) have to be "abused" in order to
work with cell_misc, by passing cell_misc/<mapname> for the "element"
parameter and <element> for the "name" parameter.

Because the name isn't actually being passed as the name, but is
embedded in the element parameter, the code which strips the @mapset
part never gets applied. When using G_open_* correctly (i.e. passing
the name as the "name" parameter), this isn't an issue, as the name
will be normalised (split it into an unqualified name and a separate
mapset) automatically.

This doesn't affect all modules: only those which fail to normalise
the map name are affected.

This has only become an issue with the recent change because,
previously, almost any libgis function which accepted a map/mapset
pair would end up normalising the name as a side-effect.

Essentially, modules which use G_find_cell2() instead of G_find_cell()
are affected. AFAICT, modules appear to be almost evenly divided
between the use of G_find_cell() and G_find_cell2().

Now, a question:

Should I leave these modules alone, in order to identify other
functions which don't like being passed qualified map names, or should
I just fix the modules?

The former is better in the long run[1], but means that, in the short
term, we will probably keep bumping into cases where qualified map
names fail.

[1] Ultimately, I hope to remove all knowledge of mapsets from the
modules. Modules should just be able to pass opt->answer directly to
libgis functions without having to deal with G_find_cell(), mapset
parameters and the like.

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