[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