[GRASS-dev] Re: [GRASS GIS] #197: sfd support for r.terraflow
GRASS GIS
trac at osgeo.org
Sat Jun 28 14:08:34 EDT 2008
#197: sfd support for r.terraflow
--------------------------+-------------------------------------------------
Reporter: adanner | Owner: grass-dev at lists.osgeo.org
Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: Raster | Version: svn-develbranch6
Resolution: | Keywords: terraflow r.terraflow
Platform: Unspecified | Cpu: Unspecified
--------------------------+-------------------------------------------------
Comment (by adanner):
Replying to [comment:4 glynn]:
> Personally, I wouldn't be especially concerned about the warnings.
Especially as they are quite legitimate; those fields should really be
"const char *", as they normally get assigned from string literals.
Although string literals are treated as "char *", on any platform we care
about, you will get a segfault if you try to modify them.
>
> Moving the code from C++ to C is simply hiding a legitimate issue. The
compiled code will be essentially the same regardless of language, and the
problems are the same. It's just that, for historical reasons, C doesn't
complain while C++ does.
>
> My attitude to warnings is to either fix them for real or leave them
there, with "hiding" them being the worst choice.
I did fix a few C++ warnings correctly in the C++ code using either
std::string or appropriate use of const. These changes were in
IOStream/include/mm_utils.h IOStream/include/mem_stream.h,
IOStream/include/ami_stream.h, and the associated .cc files. The only
C-like fixes is to hide warnings from setting fields in the Grass struct
GModule and struct Option. Every other module has similar code, but since
every other module is in C, there are no warnings. I'm not hiding anything
terrafow related in this .c file.
If you want, I can throw the code from tflowopt.c back into main.cc and
then you'll get 30+ warning about how terraflow is trying to connect to
the rest of Grass. This seems excessive, since there currently is no right
way to fix a C-style interface in C++ without bloating the code and making
it unreadable. Basically every option parameter would need to be set as
follows.
string tmp="memory";
mem->key=tmp.c_str();
tmp="300"
mem->answer=tmp.c_str();
Blech. I personally think the .c file is the right way to have C++ code
talk to the few C-style structs it needs to initialize parameters.
I'd happily push the code back into main.cc, but it would be nice to get a
patch committed. I've received email about this error at least three times
over the past few years. Just let me know if you want me to revert the
tflowopts changes.
--
Ticket URL: <http://trac.osgeo.org/grass/ticket/197#comment:5>
GRASS GIS <http://grass.osgeo.org>
More information about the grass-dev
mailing list