<p dir="ltr">I think in the long term it would be better to change the raster directory structure such that raster and vector have the same mapset/(raster, vector)/mapname/element path. Then we would be able to move some useful element related functions from librast to libgis.</p>

<p dir="ltr">Huidae</p>
<div class="gmail_quote">On May 13, 2014 5:11 PM, "Glynn Clements" <<a href="mailto:glynn@gclements.plus.com">glynn@gclements.plus.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Huidae Cho wrote:<br>
<br>
> I agree. Maybe, Markus M has a better idea why we needed r60149?<br>
<br>
AFAICT, the reason for the logic change is so that it doesn't print a<br>
warning if the file simply doesn't exist.<br>
<br>
The previous version would only call G_find_file2() if the mapset<br>
couldn't otherwise be determined, i.e. the name was unqualified and no<br>
mapset was given. If the mapset could be determined either from a<br>
qualified name or a non-empty mapset parameter, a non-existent file<br>
wouldn't be detected until the open() call failed.<br>
<br>
The new version always calls G_find_file2() first, so open() should<br>
never be called for a non-existent file (or one which is inaccessible<br>
due to the current process lacking execute (search) permission on any<br>
ancestor directory). So a failed open() would indicate something less<br>
mundane than a missing file.<br>
<br>
I'm assuming that some code used the functions for a combined "check<br>
whether it exists, and if so, open it" function. If that's the case,<br>
warning about non-existent files would be a nuisance.<br>
<br>
I'm still not sure whether this behaviour is a good idea, or whether<br>
we should always generate the warning and require callers to perform<br>
an explicit G_find_* call if the file's existence is merely "useful"<br>
rather than either "required" or at least "expected".<br>
<br>
And none of this changes the fact that Vect_read_colors() was broken<br>
prior to r60225, and is still broken now.<br>
<br>
If a function has parameters which happen to be used as the various<br>
components of a filename, code shouldn't assume that they're just<br>
going to get glued together and that it doesn't matter about the<br>
individual pieces.<br>
<br>
It's still broken now because the first argument to Rast__read_colors<br>
is supposed to be an "element name", not two directory names glued<br>
together. That it happens to work as a coincidence of implementation<br>
details doesn't make it correct (in fact, "happens to work as a<br>
coincidence of implementation details" is pretty much the definition<br>
of a "hack").<br>
<br>
Given that the current prototype for Rast__read_colors() doesn't<br>
actually support reading colours for a vector map (because of the<br>
vector code using the <mapset>/vector/<map>/<element> layout rather<br>
than <mapset>/<element>/<map>), the correct solution is to add a new<br>
function which takes a FILE* and let Vlib deal with opening (and<br>
closing) the file.<br>
<br>
--<br>
Glynn Clements <<a href="mailto:glynn@gclements.plus.com">glynn@gclements.plus.com</a>><br>
</blockquote></div>