[GRASS-dev] clean_temp rewritten

GRASS GIS GRASS GIS developer
1

Hi,

I have (after a while of testing) submitted the new
lib/init/clean_temp.c code from Roberto Flor (ITC-irst).

The new code has better timing to remove old leftover
tmp files and also removed leftover subdirectories now
(in LOCATION/MAPSET/.tmp/).

Here some related offlist discussion excepts from the
test phase of the new version:

On Tue, May 16, 2006 at 04:20:43AM +0100, Glynn Clements wrote:

Markus Neteler wrote:
> Markus Neteler wrote:
> > Glynn Clements wrote:
> > > if ( (pathlen=snprintf(buf,BUF_MAX,"%s/%s",pathname,cur_entry->d_name)) >= BUF_MAX)
> > > G_fatal_error("clean_temp: exceeded maximum pathname length %d, got %d, should'nt happen",BUF_MAX,pathlen);
> >
> > Unfortunately:
> >
> > 1. snprintf() is C99; not all systems have it.
>
> So far it is implemented a few times in GRASS:
>
> find . -type f -name "*.c" -exec grep -l snprintf {} \;
> ./raster3d/r3.in.ascii/main.c
> ./db/drivers/dbf/dbfexe.c
> ./raster/r.support/front/front.c
> ./raster/r.support/front/check.c
> ./raster/r.support/front/run.c
> ./raster/r.support/modhead/check_un.c
> ./raster/r.support/modhead/modhead.c
> ./raster/r.support/modhead/ask_format.c
> ./lib/db/sqlp/lex.yy.c
> ./lib/db/dbmi_client/select.c
> ./lib/gis/user_config.c
> ./lib/vector/dglib/examples/components.c
>
> Should this become at least a G_snprintf() wrapper
> in libgis?

I'm not sure.

If libc doesn't provide snprintf, the only options are to:

1. Implement it from scratch.
2. Write a wrapper which uses fprintf() to write the string to a
temporary file then reads the temporary file into the buffer.
3. Write a wrapper which discards the size argument and calls
vsprintf().

Option 1 is a fair amount of work (even extracting the code from libc
isn't simple). Option 2 is messy and option 3 creates the risk of
buffer overruns.

This issue has been discussed on the list in the past; IIRC, the
discussion was initiated by someone trying to compile GRASS on a
system which lacked snprintf(). I don't recall the conclusion.

This was also Glynn's suggestion in 2002 AFAIK:
http://grass.itc.it/pipermail/grass5/2002-April/005218.html
"Should I add a stub implementation to libgis?"

Here is the full thread:
http://grass.itc.it/pipermail/grass5/2002-April/thread.html#5117
"snprintf and other compile errors on IRIX"

IMHO, a quick solution to the problem is:
- make G_snprintf() a wrapper around snprintf()
- test therein if the system support snprintf(), if not, issue
  an error (or write an implementation, see below).

In this case we could identify systems with snprintf() being
unsupported using G_snprintf() and add extra tricks.

BTW, here a thread on mingW:
http://lists-archives.org/mingw-users/03714-mingw-snprintf-not-following-c99-standard.html

See here for a GPL'ed snprintf.c (suggested in above mingW thread):
http://www.ijs.si/software/snprintf/

Markus

2

Hi,

a recent change makes i.find segfault.

http://freegis.org/cgi-bin/viewcvs.cgi/grass6/imagery/i.find/main.c.diff?r1=2.4&r2=2.5

e.g. startup i.vpoints, click "vector" and you'll get a G_fatal_error():
ERROR: ask_gis_files: can't read tempfile
  [from i.vpoints/ask.c]

as i.find segfaulted didn't create the tempfile.

I've just added a return value test to the system("i.find") call in
i.vpoints (find.c), so you won't see that error after a cvs update.

it seems to break here:
  imagery/i.find/main.c line 111, in find()

while ((dp = readdir(dfd)) != NULL)

usage: (writes elements to a file)
  i.find [location] [mapset] [element] [output file]

e.g. list of spearfish raster maps:

G61> $GISBASE/etc/i.find spearfish60 PERMANENT cell /tmp/cellmaps.txt
Segmentation fault

thanks,
Hamish

3

Hello Hamish/Brad
I've fixed this now:

On Tue, 27 Jun 2006, Hamish wrote:

Hi,

a recent change makes i.find segfault.

[...]

it seems to break here:
imagery/i.find/main.c line 111, in find()

while ((dp = readdir(dfd)) != NULL)

It was actually a bit before that around:
    G__file_name (dir, element, "", mapset);
    if ((dfd = opendir(dir)) == NULL)
        continue;
With the recent changes, dir was an uninitialised pointer at this point but G__file_name() was expecting a buffer into which it could copy its result.

Paul

4

On Tue, 27 Jun 2006 15:31:14 +0100 (BST)
Paul Kelly <paul-grass@stjohnspoint.co.uk> wrote:

Hello Hamish/Brad
I've fixed this now:

On Tue, 27 Jun 2006, Hamish wrote:

> Hi,
>
> a recent change makes i.find segfault.
>
[...]
>
> it seems to break here:
> imagery/i.find/main.c line 111, in find()
>
> while ((dp = readdir(dfd)) != NULL)

It was actually a bit before that around:
    G__file_name (dir, element, "", mapset);
    if ((dfd = opendir(dir)) == NULL)
        continue;
With the recent changes, dir was an uninitialised pointer at this
point but G__file_name() was expecting a buffer into which it could
copy its result.

thanks, works.

One more problem, it now lists "." and ".." as filenames in the element
list. Obviously not maps..

Hamish

5

Hello Hamish

On Wed, 28 Jun 2006, Hamish wrote:

thanks, works.

One more problem, it now lists "." and ".." as filenames in the element
list. Obviously not maps..

Now it ignores hidden files (starting with a .). Does it work OK now? I notice the filenames aren't sorted as they were in the old version.

Paul

6

Paul:

Now it ignores hidden files (starting with a .). Does it work OK now? I
notice the filenames aren't sorted as they were in the old version.

Yes, in mild testing it seems to work fine.

sorting code in g.mlist?

thanks,
Hamish