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