[GRASS-dev] etc file finder, take 2

(ooh, my brain hurts after this - not made for C)

Here's what I came up with. It uses an env var, GRASS_ADDON_ETC, much like the PATH and GRASS_ADDON_PATH vars - a colon-delimited list of paths to look in. And finally checks the GRASS application etc/.

It returns the full path to the found file or folder, or null if not found.

(attachments)

find_etc.c (70 Bytes)
find_etc.c (1.36 KB)
main.c (1.21 KB)

William Kyngesburye wrote:

Here's what I came up with. It uses an env var, GRASS_ADDON_ETC,
much like the PATH and GRASS_ADDON_PATH vars - a colon-delimited list
of paths to look in. And finally checks the GRASS application etc/.

It returns the full path to the found file or folder, or null if not
found.

The mechanism seems reasonable, although I have quite a few comments
on the implementation.

The main one is that I'd suggest using G_tokenize() to do most of the
work for you. Also, make the argument "const char *" (you don't need
to modify it) and use GPATH_MAX as the size of the path buffer (I've
been replacing various random constants with that value as I find
them; also for GNAME_MAX and GMAPSET_MAX)..

E.g. (untested):

static char *G__find_etc(const char *name)
{
  const char *pathlist = getenv("GRASS_ADDON_ETC");
  char path[GPATH_MAX];

  if (pathlist)
  {
    char **dirs = G_tokenize(pathlist, ":");
    char *result = NULL;
    int i;

    for (i = 0; dirs[i]; i++)
    {
      sprintf(path, "%s/%s", dirs[i], name);

      if (access(path, 0) == 0)
      {
        result = G_store(path);
        break;
      }
    }
  
    G_free_tokens(dirs);
  
    if (result)
      return result;
  }

  sprintf(path, "%s/etc/%s", G_gisbase(), name);
  if (access(path, 0) == 0)
    return G_store(path);

  return NULL;
}

Also:

#include <grass/etc.h>

Not needed; add it to gisdefs.h.

And a companion g.findetc for use in scripts:

  exit(fpath==NULL);

  exit(fpath ? EXIT_SUCCESS : EXIT_FAILURE);

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

Thanks...

On Apr 18, 2007, at 11:16 AM, Glynn Clements wrote:

The main one is that I'd suggest using G_tokenize() to do most of the
work for you.

(more C details that escape me...)

My little (basic) C book says a program shouldn't alter an env var returned by getenv, and G_tokenize() says it changes the token to null to terminate token strings (which is what I originally did myself before I read that bit about getenv). Is that what that char ** or const char * takes care of somehow, do either make a copy of the env var? Or is it part of the tokenize initialization?

Also, make the argument "const char *" (you don't need
to modify it) and use GPATH_MAX as the size of the path buffer (I've
been replacing various random constants with that value as I find
them; also for GNAME_MAX and GMAPSET_MAX)..

Ah, I used an older find_file as a starting point. I see you updated this yesterday.

E.g. (untested):

static char *G__find_etc(const char *name)
{
  const char *pathlist = getenv("GRASS_ADDON_ETC");
  char path[GPATH_MAX];

...

}

I'll give this a closer look and try it this evening.

Also:

#include <grass/etc.h>

Not needed; add it to gisdefs.h.

Oops, leftover from the earlier configured-paths version. Not needed now. But, I do have the prototype for G_find_etc() in gisdefs.h.

And a companion g.findetc for use in scripts:

  exit(fpath==NULL);

  exit(fpath ? EXIT_SUCCESS : EXIT_FAILURE);

got it.

-----
William Kyngesburye <kyngchaos*at*kyngchaos*dot*com>
http://www.kyngchaos.com/

[Trillian] What are you supposed to do WITH a maniacally depressed robot?

[Marvin] You think you have problems? What are you supposed to do if you ARE a maniacally depressed robot? No, don't try and answer, I'm 50,000 times more intelligent than you and even I don't know the answer...

- HitchHiker's Guide to the Galaxy

William Kyngesburye wrote:

> The main one is that I'd suggest using G_tokenize() to do most of the
> work for you.

(more C details that escape me...)

My little (basic) C book says a program shouldn't alter an env var
returned by getenv, and G_tokenize() says it changes the token to
null to terminate token strings (which is what I originally did
myself before I read that bit about getenv). Is that what that char
** or const char * takes care of somehow, do either make a copy of
the env var? Or is it part of the tokenize initialization?

G_tokenize() makes a copy of the string.

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

That all works nicely. thanks.

For the const char * arg in G__find_etc() - should I also do that for the public version of this, G_find_etc()?

On Apr 18, 2007, at 11:16 AM, Glynn Clements wrote:

work for you. Also, make the argument "const char *" (you don't need
to modify it) and use GPATH_MAX as the size of the path buffer (I've
been replacing various random constants with that value as I find
them; also for GNAME_MAX and GMAPSET_MAX)..

E.g. (untested):

static char *G__find_etc(const char *name)
{

-----
William Kyngesburye <kyngchaos*at*kyngchaos*dot*com>
http://www.kyngchaos.com/

"Mon Dieu! but they are all alike. Cheating, murdering, lying, fighting, and all for things that the beasts of the jungle would not deign to possess - money to purchase the effeminate pleasures of weaklings. And yet withal bound down by silly customs that make them slaves to their unhappy lot while firm in the belief that they be the lords of creation enjoying the only real pleasures of existence....

- the wisdom of Tarzan

William Kyngesburye wrote:

That all works nicely. thanks.

For the const char * arg in G__find_etc() - should I also do that for
the public version of this, G_find_etc()?

Yes. When passing a pointer as an argument, if the function doesn't
modify the data, the "const" qualifier should be used.

I've recently gone through libgis and added "const" wherever it is
applicable, so if a libgis function *doesn't* use "const" for a
pointer argument, then it modifies (or may modify) the value.

Eventually, I'll get around to doing the same for other libraries.

Note that certain structure types almost never have the "const"
qualifier, even for "read" operations, e.g. the Categories, Colors,
and Quant structures. This is because they contain fields which are
initialised "on demand". E.g. a call to G_get_color() etc will
eventually call G__organize_colors(), which initialises the lookup
tables if they haven't already been initialised.

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

Glynn Clements wrote:

use GPATH_MAX as the size of the path buffer (I've
been replacing various random constants with that value as I find
them; also for GNAME_MAX and GMAPSET_MAX)..

more of a comment than a question-

"char input_map[GNAME_MAX];" is often used to hold input=map@mapset
name, when the array often should be able to hold "GNAME_MAX + @ +
GMAPSET_MAX + \0" = 514 chars.

GNAME_MAX is long enough (256, gis.h) that map@mapset should rarely
exceed that, but it's something to look out for.

Perhaps gis.h should also have G_FULLYQUALIFIED_MAX ?

Hamish

Hamish wrote:

> use GPATH_MAX as the size of the path buffer (I've
> been replacing various random constants with that value as I find
> them; also for GNAME_MAX and GMAPSET_MAX)..

more of a comment than a question-

"char input_map[GNAME_MAX];" is often used to hold input=map@mapset
name, when the array often should be able to hold "GNAME_MAX + @ +
GMAPSET_MAX + \0" = 514 chars.

GNAME_MAX is long enough (256, gis.h) that map@mapset should rarely
exceed that, but it's something to look out for.

Perhaps gis.h should also have G_FULLYQUALIFIED_MAX ?

Who said that GNAME_MAX referred to the length of an *unqualified*
name?

It's just an arbitrary value used for array dimensions; nothing
actually checks map names against that value.

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

Hamish wrote:
> > use GPATH_MAX as the size of the path buffer (I've
> > been replacing various random constants with that value as I find
> > them; also for GNAME_MAX and GMAPSET_MAX)..
>
> more of a comment than a question-
>
> "char input_map[GNAME_MAX];" is often used to hold input=map@mapset
> name, when the array often should be able to hold "GNAME_MAX + @ +
> GMAPSET_MAX + \0" = 514 chars.
>
> GNAME_MAX is long enough (256, gis.h) that map@mapset should rarely
> exceed that, but it's something to look out for.
>
> Perhaps gis.h should also have G_FULLYQUALIFIED_MAX ?

Glynn:

Who said that GNAME_MAX referred to the length of an *unqualified*
name?

It's just an arbitrary value used for array dimensions; nothing
actually checks map names against that value.

/* File/directory name lengths */
#define GNAME_MAX 256
#define GMAPSET_MAX 256

#define GPATH_MAX 4096

if GMAPSET_MAX is 256, and GNAME_MAX should be able to hold name@mapset,
shouldn't GNAME_MAX be at least GMAPSET_MAX+2?
[2= 1 char long mapset name + @]

otherwise concat can theoretically overflow.

(playing devil's advocate here to make the allocs a little more bad-
assumption-proof)

Hamish

Hamish wrote:

> > > use GPATH_MAX as the size of the path buffer (I've
> > > been replacing various random constants with that value as I find
> > > them; also for GNAME_MAX and GMAPSET_MAX)..
> >
> > more of a comment than a question-
> >
> > "char input_map[GNAME_MAX];" is often used to hold input=map@mapset
> > name, when the array often should be able to hold "GNAME_MAX + @ +
> > GMAPSET_MAX + \0" = 514 chars.
> >
> > GNAME_MAX is long enough (256, gis.h) that map@mapset should rarely
> > exceed that, but it's something to look out for.
> >
> > Perhaps gis.h should also have G_FULLYQUALIFIED_MAX ?

Glynn:
> Who said that GNAME_MAX referred to the length of an *unqualified*
> name?
>
> It's just an arbitrary value used for array dimensions; nothing
> actually checks map names against that value.

/* File/directory name lengths */
#define GNAME_MAX 256
#define GMAPSET_MAX 256

#define GPATH_MAX 4096

if GMAPSET_MAX is 256, and GNAME_MAX should be able to hold name@mapset,
shouldn't GNAME_MAX be at least GMAPSET_MAX+2?
[2= 1 char long mapset name + @]

Not necessarily; it just means that if you have really long names[1]
for both maps and mapsets, you may not be able to use fully-qualified
names.

[1] 256 characters > 3 * 80-character lines, which is *really*
long[2].

[2] Long enough that I should probably look at changing struct Reclass
to use a dynamically-allocated char* rather than a fixed-sized array,
given that the G__ structure has one instance per descriptor.

otherwise concat can theoretically overflow.

Simply copying the map name into the array can theoretically overflow.
Nothing prevents the user from specifying a map name which exceeds
GNAME_MAX (likewise for mapsets).

(playing devil's advocate here to make the allocs a little more bad-
assumption-proof)

The actual values don't really matter unless something actually checks
them. The main advantage of pre-defined constants is that it
discourages authors from using excessively small buffers.

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