Fix the issue of returning a pointer to a local variable in data_products

[hchen99]

Fix the issue of returning a pointer to a local variable in data_products

[dandexter]

There are some issues with the updated code.

1) For example, this line from "trick_source/data_products/DPX/APPS/FXPLOT/plot_view_node.cpp" if ((const_temp_str = plot->getXLabel().c_str()) != NULL) { Because the type is an str::string, the c_str() will never be NULL so the code should take that into account.

2) For file "trick_source/data_products/DPX/APPS/FXPLOT/table_view_node.cpp" this code segment is convert to a c_str() to check for NULL. Code should be refactored to embrace using the str::string type. const char column_label = table->getColumnLabel( colix); const char column_label = table->getColumnLabel( colix).c_str(); if (!columnlabel) { snprintf( charbuf, sizeof(charbuf), "Column%d", colix);

3) Why use c-code API (strdup) on the y_actual_units variable of type stl::string in file "trick_source/data_products/DPX/DPC/DPC_std_curve.cpp"? std::string ds_units = ds[0]->getUnit(); y_actual_units = strdup(ds_units); y_actual_units = strdup(ds_units.c_str());

4) Why use c-code API's on str::string type. Should be using c++ str::string functions instead in file "trick_source/data_products/DPX/DPM/DPM_curve.cpp" in changes made.

5) Unnecessary check for NULL c_str() in file "trick_source/data_products/DPX/DPM/DPM_relation.cpp":

     if (( short_name = curve_list[i]->getXCommonName().c_str() ) == NULL ) {
           return ("");
      }

6) Unnecessary converting string to c_str() for stream processing in file "trick_source/data_products/DPX/test/unit_test/test_view.cpp". Just send the string to the output without converting to a c_str() s << "X-axis label: " << plot->getXLabel().c_str() << std::endl; s << "Y-axis label: " << plot->getYLabel().c_str() << std::endl;

Also, unnecessary check for NULL if ((temp_cstr = table->getColumnLabel(colix).c_str()) != NULL) {

[hchen99]

There are some issues with the updated code.

1. For example, this line from "trick_source/data_products/DPX/APPS/FXPLOT/plot_view_node.cpp" if ((const_temp_str = plot->getXLabel().c_str()) != NULL) { Because the type is an str::string, the c_str() will never be NULL so the code should take that into account.
2. For file "trick_source/data_products/DPX/APPS/FXPLOT/table_view_node.cpp" this code segment is convert to a c_str() to check for NULL. Code should be refactored to embrace using the str::string type. const char column_label = table->getColumnLabel( colix); const char column_label = table->getColumnLabel( colix).c_str(); if (!columnlabel) { snprintf( charbuf, sizeof(charbuf), "Column%d", colix);
3. Why use c-code API (strdup) on the y_actual_units variable of type stl::string in file "trick_source/data_products/DPX/DPC/DPC_std_curve.cpp"? std::string ds_units = ds[0]->getUnit(); y_actual_units = strdup(ds_units); y_actual_units = strdup(ds_units.c_str());
4. Why use c-code API's on str::string type. Should be using c++ str::string functions instead in file "trick_source/data_products/DPX/DPM/DPM_curve.cpp" in changes made.
5. Unnecessary check for NULL c_str() in file "trick_source/data_products/DPX/DPM/DPM_relation.cpp":

    if (( short_name = curve_list[i]->getXCommonName().c_str() ) == NULL ) {
          return ("");
     }

6. Unnecessary converting string to c_str() for stream processing in file "trick_source/data_products/DPX/test/unit_test/test_view.cpp". Just send the string to the output without converting to a c_str() s << "X-axis label: " << plot->getXLabel().c_str() << std::endl; s << "Y-axis label: " << plot->getYLabel().c_str() << std::endl;

Also, unnecessary check for NULL if ((temp_cstr = table->getColumnLabel(colix).c_str()) != NULL) {

Thanks for your helpful feedback. Will get them addressed!

Item#3: x_actual_units and y_actual_units are defined as char * not std::string. So didn't make the update ;-)

[dandexter]

I did not check all the code but you may want to do a grep for "c_str()" so you can review all the places it shows up in the Data Products code to make sure you have resolved all the checks against NULL.

Just looking at the code diff's I think this one got missed. File: trick_source/data_products/DPX/DPM/DPM_relation.cpp Line: 204 if (( short_name = curve_list[i]->getYCommonName().c_str() ) == NULL ) {

[hchen99]

I did not check all the code but you may want to do a grep for "c_str()" so you can review all the places it shows up in the Data Products code to make sure you have resolved all the checks against NULL.

Just looking at the code diff's I think this one got missed. File: trick_source/data_products/DPX/DPM/DPM_relation.cpp Line: 204 if (( short_name = curve_list[i]->getYCommonName().c_str() ) == NULL ) {

Thanks for catching this Dan! Fixed Y&Z for this file. Will double check if there is anything else. Also, we'll discuss how we want to handle code refactoring for data_products. Thanks again!

[coveralls]

coverage: 55.746%. remained the same when pulling 56ba67016d93ed523f79c9ebc1cbae51edc268d9 on 1560-returning-a-pointer-to-a-local-variable into d2b699ce04c028a581b96802983374a833d022eb on master.