[GRASS-dev] G__ in public API

Hi all,

many of modules are using G__ functions which should be dedicated for
internal use only (in libs level). Those which are used by modules I
would suggest to rename to G_ (before GRASS 7 release). Any
objections, comments?

Martin

--
Martin Landa * http://geo.fsv.cvut.cz/gwiki/Landa

On Tue, Jun 10, 2014 at 4:12 AM, Martin Landa <landa.martin@gmail.com>
wrote:

Hi all,

many of modules are using G__ functions which should be dedicated for
internal use only (in libs level). Those which are used by modules I
would suggest to rename to G_ (before GRASS 7 release). Any
objections, comments?

I agree that something must be done. It took me a while before I

understood that double underscore methods are meant to be private (because
of lack of documentation and bad examples in modules' code).

Vaclav

Martin

--
Martin Landa * http://geo.fsv.cvut.cz/gwiki/Landa
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev

I agree. One of the main reasons why G__ routines are used is there is no alternative ways to achieve certain tasks. Then they should be exposed to module developers. If it’s not the case, those routines have to replaced with correct G_ versions.

For example, I recently used G__mapset_permissions in g.mlist.

···

On Tue, Jun 10, 2014 at 4:12 AM, Martin Landa <landa.martin@gmail.com> wrote:

Hi all,

many of modules are using G__ functions which should be dedicated for
internal use only (in libs level). Those which are used by modules I
would suggest to rename to G_ (before GRASS 7 release). Any
objections, comments?

I agree that something must be done. It took me a while before I understood that double underscore methods are meant to be private (because of lack of documentation and bad examples in modules’ code).

Vaclav

Martin


Martin Landa * http://geo.fsv.cvut.cz/gwiki/Landa


grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev

On Tue, Jun 10, 2014 at 9:56 AM, Huidae Cho <grass4u@gmail.com> wrote:

I agree. One of the main reasons why G__ routines are used is there is no
alternative ways to achieve certain tasks. Then they should be exposed to
module developers. If it's not the case, those routines have to replaced
with correct G_ versions.

For example, I recently used G__mapset_permissions in g.mlist.

This might be a part of advanced API, but it is still an API, it is not
internal.

On Jun 10, 2014 9:33 AM, "Vaclav Petras" <wenzeslaus@gmail.com> wrote:

On Tue, Jun 10, 2014 at 4:12 AM, Martin Landa <landa.martin@gmail.com>
wrote:

Hi all,

many of modules are using G__ functions which should be dedicated for
internal use only (in libs level). Those which are used by modules I
would suggest to rename to G_ (before GRASS 7 release). Any
objections, comments?

I agree that something must be done. It took me a while before I

understood that double underscore methods are meant to be private (because
of lack of documentation and bad examples in modules' code).

Vaclav

Martin

--
Martin Landa * http://geo.fsv.cvut.cz/gwiki/Landa
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev

_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev

Martin Landa wrote:

many of modules are using G__ functions which should be dedicated for
internal use only (in libs level). Those which are used by modules I
would suggest to rename to G_ (before GRASS 7 release). Any
objections, comments?

It's probably an exaggeration to say that G__ functions are "internal
use only". It's closer to "this is (probably) not the function you
should be using". Typically, it indicates a lower-level function which
is "primarily" for internal use.

There does seem to be some excessive use of G__ names (e.g.
lib/gis/env.c). But I wouldn't go so far as to say that G__ names
should *never* be used for functions which are used externaly,
particularly if such use is limited to a few specialised modules.

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

2014-06-11 13:00 GMT+02:00 Glynn Clements <glynn@gclements.plus.com>:

It's probably an exaggeration to say that G__ functions are "internal
use only". It's closer to "this is (probably) not the function you
should be using". Typically, it indicates a lower-level function which
is "primarily" for internal use.

There does seem to be some excessive use of G__ names (e.g.
lib/gis/env.c). But I wouldn't go so far as to say that G__ names
should *never* be used for functions which are used externaly,
particularly if such use is limited to a few specialised modules.

sure, but there are not few...

$ grep 'G__' * -R --include=*.c | grep -v ^lib | cut -d: -f1 | cut -d
'/' -f2 | sort | uniq | wc -l
40

Martin

--
Martin Landa * http://geo.fsv.cvut.cz/gwiki/Landa

Martin Landa wrote:

2014-06-11 13:00 GMT+02:00 Glynn Clements <glynn@gclements.plus.com>:

> It's probably an exaggeration to say that G__ functions are "internal
> use only". It's closer to "this is (probably) not the function you
> should be using". Typically, it indicates a lower-level function which
> is "primarily" for internal use.
>
> There does seem to be some excessive use of G__ names (e.g.
> lib/gis/env.c). But I wouldn't go so far as to say that G__ names
> should *never* be used for functions which are used externaly,
> particularly if such use is limited to a few specialised modules.

sure, but there are not few...

$ grep 'G__' * -R --include=*.c | grep -v ^lib | cut -d: -f1 | cut -d
'/' -f2 | sort | uniq | wc -l
40

Looking at which functions are used, and how often:

     36 G__getenv
     23 G__setenv
     15 G__mapset_permissions
      9 G__put_window
      8 G__freea
      8 G__alloca
      6 G__getenv2
      5 G__projection_name
      5 G__get_window
      3 G__write_timestamp
      3 G__temp_element
      3 G__ls
      2 G__tempfile
      2 G__switch_search_path
      1 G__mapset_permissions2
      1 G__make_mapset_element

In most cases, the double-underscore version is a lower-level function
which takes more arguments. The corresponding single-underscore
version obtains some of the parameters from the working environment or
hard-coded defaults.

The double-underscore versions could be renamed to indicate what they
don't do, e.g.

  G__getenv -> G_getenv_no_error
  G__setenv -> G_setenv_no_write
  G__tempfile -> G_tempfile_for_pid
  G__ls -> G_ls_raw

Some of them (e.g. G__mapset_permissions) don't have a
single-underscore version, but aren't intended to be used by normal
modules. The extra underscore could simply be removed.

G__freea and G__alloca aren't functions but macros, defined thus:

  #ifdef __GNUC__
  ...
  # define G__alloca(n) alloca(n)
  # define G__freea(p)
  #else
  # define G__alloca(n) G_malloc(n)
  # define G__freea(p) G_free(p)
  #endif

In particular, G__alloca() can't be a function, as the pointer
returned by alloca() is only valid until the calling function returns
(it allocates space on the stack).

These could either keep the extra underscore or changed to
all-uppercase as per the normal macro convention (G_ALLOCA, G_FREEA).

Finally, there are a few cases (which won't show up in source code
analysis) where the single-underscore name is a macro which expands to
a call to a function with a double-underscore name, e.g.

  #define G_malloc(n) G__malloc(__FILE__, __LINE__, (n))
  #define G_calloc(m, n) G__calloc(__FILE__, __LINE__, (m), (n))
  #define G_realloc(p, n) G__realloc(__FILE__, __LINE__, (p), (n))

  #define G_gisinit(pgm) G__gisinit(GIS_H_VERSION, (pgm))
  #define G_no_gisinit() G__no_gisinit(GIS_H_VERSION)

The linkage database generated by tools/dep_tree2sql.sh will show the
double-underscore names.

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