[r.average]
should G_tempfile() be calling G_convert_dirseps_to_host() internally,
rather than having individual modules calling it? (ie often forgetting
to call it)
Hamish
On Mon, 18 Jun 2007, Hamish wrote:
[r.average]
should G_tempfile() be calling G_convert_dirseps_to_host() internally,
rather than having individual modules calling it? (ie often forgetting
to call it)
I don't think so - G_convert_dirseps_to_host() only needs to be called in certain situations, when the generated path will be used directly by external commands on the system. I think to unconditionally call it would confuse its purpose. But then again in the longrun it might be a good idea to generally simplify things as you said. Hmm, I'm not so sure now. But there may be cases which call it and expect to have '/' directory separators there for some further processing; I wouldn't like to just change it without an audit of where it is used. Which modules were you thinking of anyway?
FWIW I've attached my proposed changes to lib/gis/tempfile.c that I still have in my local source tree. They were intended to split the usage of G_tempfile() between modules that are genuinely using it as a tempfile for creating large temporary maps as was originally intended, and those that are just using it as a general small tempfile function. They also make some changes to make it more Windows compatible. But we decided they were too controversial to commit in case they broke things - maybe in some way we can eventually merge them though.
Paul
On Mon, 18 Jun 2007, Paul Kelly wrote:
On Mon, 18 Jun 2007, Hamish wrote:
[r.average]
should G_tempfile() be calling G_convert_dirseps_to_host() internally,
rather than having individual modules calling it? (ie often forgetting
to call it)I don't think so - G_convert_dirseps_to_host() only needs to be called in certain situations, when the generated path will be used directly by external commands on the system. I think to unconditionally call it would confuse its purpose. But then again in the longrun it might be a good idea to generally simplify things as you said. Hmm, I'm not so sure now. But there may be cases which call it and expect to have '/' directory separators there for some further processing; I wouldn't like to just change it without an audit of where it is used. Which modules were you thinking of anyway?
FWIW I've attached my proposed changes to lib/gis/tempfile.c that I still
^^^^^^^^
Oops - didn't even upload the diff never mind attach it. But here it is now if anyone is interested. Interestingly, the patch forces all generated filenames to have '/' directory separators - presumably because I was concerned that some modules might expect this, although I can't remember to be sure...
have in my local source tree. They were intended to split the usage of G_tempfile() between modules that are genuinely using it as a tempfile for creating large temporary maps as was originally intended, and those that are just using it as a general small tempfile function. They also make some changes to make it more Windows compatible. But we decided they were too controversial to commit in case they broke things - maybe in some way we can eventually merge them though.
Paul
_______________________________________________
grass-dev mailing list
grass-dev@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass-dev
(attachments)
diff.txt (6.32 KB)
Paul Kelly wrote:
> should G_tempfile() be calling G_convert_dirseps_to_host()
> internally, rather than having individual modules calling it? (ie
> often forgetting to call it)I don't think so - G_convert_dirseps_to_host() only needs to be called
in certain situations, when the generated path will be used directly
by external commands on the system. I think to unconditionally call
it would confuse its purpose. But then again in the longrun it might
be a good idea to generally simplify things as you said. Hmm, I'm not
so sure now. But there may be cases which call it and expect to have
'/' directory separators there for some further processing; I
wouldn't like to just change it without an audit of where it is used.
Which modules were you thinking of anyway?
I just added a G_tempfile() call in d.title, now with the -d flag it
does fopen(),fclose() then like G_system("d.text < \"%s\"", tmpfile).
Hamish
On 18/06/07 13:02, Hamish wrote:
I just added a G_tempfile() call in d.title, now with the -d flag it
does fopen(),fclose() then like G_system("d.text < \"%s\"", tmpfile).
Just wondering: how Windows-friendly is such usage of G_system ?
Moritz
On Mon, 18 Jun 2007, Moritz Lennert wrote:
On 18/06/07 13:02, Hamish wrote:
I just added a G_tempfile() call in d.title, now with the -d flag it
does fopen(),fclose() then like G_system("d.text < \"%s\"", tmpfile).Just wondering: how Windows-friendly is such usage of G_system ?
I think it should be fine. cmd.exe understands the redirection operators like <, > and |. And using double quotes to quote pathnames is also Windows-compatible (single quotes won't work).
Paul
Paul Kelly wrote:
FWIW I've attached my proposed changes to lib/gis/tempfile.c that I still
have in my local source tree. They were intended to split the usage of
G_tempfile() between modules that are genuinely using it as a tempfile for
creating large temporary maps as was originally intended, and those that
are just using it as a general small tempfile function. They also make
some changes to make it more Windows compatible. But we decided they were
too controversial to commit in case they broke things - maybe in some way
we can eventually merge them though.
A safe starting point would be to:
1. Rename G__tempfile() to G__tempfile_in_mapset().
2. Add:
char *G__tempfile(int pid)
{
return G__tempfile_in_mapset(pid);
}
3. Change opencell.c (and anything else known to require the existing
behaviour) to use G__tempfile_in_mapset() instead of G__tempfile().
Once that's done, any new G__tempfile() implementation can easily be
reverted if it's found to cause problems.
--
Glynn Clements <glynn@gclements.plus.com>