Opened 21 months ago

Closed 21 months ago

Last modified 21 months ago

#6100 closed enhancement (fixed)

Disallow copy and assignment in OGRFeatureDefn

Reported by: Kurt Schwehr Owned by: warmerdam
Priority: normal Milestone: 2.1.0
Component: default Version: svn-trunk
Severity: normal Keywords:
Cc:

Description (last modified by Kurt Schwehr)

This is a change I'd like to add to make sure that sneaky copy or assignment usages do not creep in on OGRFeatureDefn where reference counting is happening. Any reason not to add the pre-C++11 version of restriction?

  • ogr/ogr_feature.h

     
    252252
    253253    static OGRFeatureDefn  *CreateFeatureDefn( const char *pszName = NULL );
    254254    static void         DestroyFeatureDefn( OGRFeatureDefn * );
     255
     256  private:
     257    // Disable copy and assignment to prevent reference counting problems.
     258    OGRFeatureDefn( const OGRFeatureDefn &that );  // No implementation.
     259    OGRFeatureDefn &operator=( const OGRFeatureDefn& );  // No implementation.
    255260};
    256261
    257262/************************************************************************/

Someday when C++11 is allowed, this would be

OGRFeatureDefn( const OGRFeatureDefn &that ) = delete;
OGRFeatureDefn &operator=( const OGRFeatureDefn& ) = delete;

Which could be behind an if:

#if __cplusplus >= 201103L
    OGRFeatureDefn( const OGRFeatureDefn &that ) = delete;
    OGRFeatureDefn &operator=( const OGRFeatureDefn & ) = delete;
#else
  private:
    // Disable copy and assignment to prevent reference counting problems.
    OGRFeatureDefn( const OGRFeatureDefn &that );  // No implementation.
    OGRFeatureDefn &operator=( const OGRFeatureDefn & );  // No implementation.
#endif

http://stackoverflow.com/questions/6077143/disable-copy-constructor

Change History (5)

comment:1 Changed 21 months ago by Kurt Schwehr

Description: modified (diff)

comment:2 Changed 21 months ago by Kurt Schwehr

Created in a disabled state: r30231

It would be great if people on non-linux platforms could give this a try. It should be cross platform.

Just flip the #if 0 to #if 1 to enable. At this point, no behavior should change.

  • port/cpl_port.h

     
    632632   Must be placed in the private section of a class and should be at the end.
    633633*/
    634634#ifdef __cplusplus
    635 #if 0 /* Initially disabled */
     635#if 1
    636636
    637637#if __cplusplus >= 201103L
    638638#define CPL_DISALLOW_COPY_ASSIGN(ClassName) \

comment:3 Changed 21 months ago by Kurt Schwehr

Enabled via r30264

comment:4 Changed 21 months ago by Even Rouault

Milestone: 2.1.0
Resolution: fixed
Status: newclosed

trunk r30277 "Avoid use of OGRFieldDefn (implicit) assignment operator"

trunk r30278 "Disable copy constructor and assignment operators in classes OGRFieldDefn, OGRGeomFieldDefn, OGRFeature, GDALMultiDomainMetadata, GDALDefaultOverviews, GDALOpenInfoGDALDataset, GDALRasterBlock, GDALRasterBand and GDALDriver (#6100)"

comment:5 Changed 21 months ago by Even Rouault

Hum, there were linking errors due to r30278 with MSVC : https://ci.appveyor.com/project/rouault/gdal-coverage/build/1.0.512/job/a0mjcqyrub6g5i8w , http://build.gisinternals.com/sdk/build-output/vc9-20150912-2-30-37-10-vc9-dev.txt

Should be fixed with r30325 "Add CPL_DISALLOW_COPY_ASSIGN to various derived classes of GDALRasterBand and GDALDataset to fix linking error with MSVC" (tested locally with VC 10 & VC 12)

I'm not sure to have really understood why this was necessary. The reasons may be given at https://msdn.microsoft.com/en-us/library/dn457344%28v=vs.120%29.aspx

Note: See TracTickets for help on using tickets.