[GRASS-dev] Re: [GRASS GIS] #718: r.li forgets mask/illegal filename

[GRASS GIS]

#718: r.li forgets mask/illegal filename
------------------------+---------------------------------------------------
  Reporter:  kyngchaos  |       Owner:  grass-dev at lists.osgeo.org
      Type:  defect     |      Status:  new                      
  Priority:  normal     |   Milestone:  6.4.0                    
 Component:  Raster     |     Version:  6.4.0 RCs                
Resolution:             |    Keywords:  r.li                     
  Platform:  MacOSX     |         Cpu:  x86-64                   
------------------------+---------------------------------------------------
Comment (by hamish):

 Replying to [comment:2 pcav]:
 > Here it is:
 > ===
 {{{
 > load landclass96
 >
 > r.li.setup
 >
 > SAMPLINGFRAME 0|0|1|1
 > SAMPLEAREA -1|-1|0.0210970464135|0.0190114068441
 > MOVINGWINDOW
 }}}
 >


 ok, I'm not familiar with the module at all so you'll have to explain it
 to me fully & slowly :)

 so far I have:
 {{{
 # nc_spm_08 NC sample dataset
 d.mon x0
 g.region rast=landclass96
 d.rast landclass96

 r.li.setup

 # GUI opens
 }}}

 now what?

 hmmm.. (guessing)
 {{{
 $ cat << EOF > ~/.r.li/history/landclass96_conf
 SAMPLINGFRAME 0|0|1|1
 SAMPLEAREA -1|-1|0.0210970464135|0.0190114068441
 MOVINGWINDOW
 EOF
 }}}

 restart r.li.setup
 {{{
 # gui pops up
 select landclass96_conf from list
 [Load]

 # gui pops up, red plot with blue sample-area cell in top left
 # "Disposition Moving Windows" (aka current moving window setup??)
 }}}

 not sure what to do next, so [Close] r.li.setup.

 {{{
 # Debian/etch i386 32bit Pentium4
 # full conf= path needed?
 G65> r.li.shannon map=landclass96 conf=landclass96_conf \
    output=landclass96_shannon
 [runs for a minute]
 r.li.worker (pid 28631) terminated
 r.li.worker (pid 28635) terminated
 r.li.worker (pid 28626) terminated
 r.li.worker (pid 28633) terminated
 r.li.worker (pid 28634) terminated
 r.li.worker (pid 28632) terminated
 r.li.worker (pid 28627) terminated
 r.li.worker (pid 28629) terminated
 r.li.worker (pid 28628) terminated
 r.li.worker (pid 28630) terminated

 G65> echo $?
 1
 }}}

 ??

 d.rast landclass96_shannon
 # looks good


 If I do the exact same steps on Debian/Lenny amd64 I get the exact same
 result.

 ?

 I would note that gkrellm on both machines shows about 1/2 the time spent
 in kernel I/O, a sign of a bad inefficiency somewhere. (also gkrellm shows
 that it is using all 4-cores on the 64bit machine, which is nice; I guess
 that is what all those workers are busy doing)
 {{{
 real    0m4.895s
 user    0m9.493s
 sys     0m9.125s

 ?  (user + sys) / num_cores = real
 }}}


 from raster/r.li/r.li.daemon/daemon.c:
 {{{
     /* TODO: check if this path is portable */
     sprintf(pathSetup, "%s/.r.li/history/%s", getenv("HOME"), file);
     parsed = parseSetup(pathSetup, l, g, raster);
 }}}

 So you should not pass the full path to `r.li.shannon conf=` as it adds it
 itself, just the config file-name. Note r.li.shannon's parser code is
 currently set up to use the file picker which fills in the full path
 automatically, so that would have to be removed or the above would need to
 test the pathname and try to chop away the $HOME/.r.li/ part if it was
 given twice. I think the r.colors rules= code in GRASS 6.5 has something
 like that for backwards compatibility - it checks in two places.

 also in grass7 that should be moved to ~/.grass/r.li/


 The pathSetup string is allocated with lots of space:
  pathSetup[GPATH_MAX]
 so I don't think the problem is you are overflowing that buffer.

 {{{
 int parseSetup(char *path, list l, g_areas g, char *raster)
 {
     struct stat s;
     struct Cell_head cellhd;
     char *buf, *token, *mapset;
     int setup;
     int letti;
 ...
     int size;

     if (stat(path, &s) != 0)
         G_fatal_error(_("Cannot make stat of %s configuration file"),
 path);
     size = s.st_size * sizeof(char);
     buf = G_malloc(size);
     setup = open(path, O_RDONLY, 0755);
     if (setup == -1)
         G_fatal_error(_("Cannot read setup file"));
     letti = read(setup, buf, s.st_size);
     if (letti < s.st_size)
         G_fatal_error(_("Cannot read setup file"));
 }}}

 probably that stat error message could be better worded..

 so the G_fatal_error() in daemon.c does not kill the process?

 ah, if I give it a garbage conf file name:
 `conf=landclass96_conf.blah` it goes nuts:

 {{{
 G65> time r.li.shannon map=landclass96 conf=landclass96_conf.blah
 output=landclass96_shannon --o
 ERROR: Cannot make stat of
        /home/hamish/.r.li/history/landclass96_conf.blah configuration file

 [--- It ''should'' quit everything at this point ---]

 Illegal filename. Character <ÿ> not allowed.
 Illegal filename. Character <ÿ> not allowed.
 Illegal filename. Character <ÿ> not allowed.
 WARNING: Unable to open header file for raster map <ÿ@(null)>
 CHILD[pid = 12064]: unable to open ÿ mask ... continue without!!!

 real    0m0.072s
 user    0m0.000s
 sys     0m0.004s
 G65> Illegal filename. Character <ÿ> not allowed.
 Illegal filename. Character <ÿ> not allowed.
 Illegal filename. Character <ÿ> not allowed.
 WARNING: Unable to open header file for raster map <ÿ@(null)>
 CHILD[pid = 12063]: unable to open ÿ mask ... continue without!!!
 Illegal filename. Character <ÿ> not allowed.
 Illegal filename. Character <ÿ> not allowed.
 Illegal filename. Character <ÿ> not allowed.
 WARNING: Unable to open header file for raster map <ÿ@(null)>
 CHILD[pid = 12062]: unable to open ÿ mask ... continue without!!!
 ...
 *** glibc detected *** r.li.shannon: corrupted double-linked list:
 0x0000000001103830 ***
 }}}


 Hamish

-- 
Ticket URL: <https://trac.osgeo.org/grass/ticket/718#comment:3>
GRASS GIS <http://grass.osgeo.org>