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
